Skip to content

Add CI job to build C sources with various compiler versions and fix array-bounds error with GCC 9#4069

Open
eregon wants to merge 4 commits intoruby:mainfrom
eregon:gcc-ci
Open

Add CI job to build C sources with various compiler versions and fix array-bounds error with GCC 9#4069
eregon wants to merge 4 commits intoruby:mainfrom
eregon:gcc-ci

Conversation

@eregon
Copy link
Copy Markdown
Member

@eregon eregon commented Apr 6, 2026

eregon added 2 commits April 6, 2026 14:37
* In file included from /usr/include/string.h:535,
                 from include/prism/internal/arena.h:12,
                 from src/prism.c:6:
  In function 'memset',
    inlined from 'lex_mode_push_regexp' at src/prism.c:290:5:
  .../string_fortified.h:59:10: error: '__builtin_memset' offset [26, 34] from the object at 'lex_mode' is out of the bounds of referenced subobject 'breakpoints' with type 'uint8_t[7]' {aka 'unsigned char[7]'} at offset 18 [-Werror=array-bounds]
@eregon eregon requested a review from kddnewton April 6, 2026 12:55
Copy link
Copy Markdown
Collaborator

@kddnewton kddnewton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the deal with the changes to parser.h here?

@eregon
Copy link
Copy Markdown
Member Author

eregon commented Apr 6, 2026

@kddnewton Could you check the commit message? It explains it.

@eregon
Copy link
Copy Markdown
Member Author

eregon commented Apr 6, 2026

The new job took 10min15s, which is faster than memcheck.
So maybe it's fast enough for now?

Not sure where the time is spent but I wondered about image size:

$ docker images
ghcr.io/ruby/ruby-ci-image                                gcc-15               771cea939adf  6 days ago      3.18 GB
docker.io/library/gcc                                     15                   9a52dbcd9b63  2 weeks ago     1.57 GB

Maybe we could use gcc:15 but then we'd also need ruby inside, or change Makefile to no longer depend on ruby for SOEXT (or just fallback to .so if no ruby).

@eregon eregon requested a review from kddnewton April 6, 2026 15:32
@eregon
Copy link
Copy Markdown
Member Author

eregon commented Apr 6, 2026

@kddnewton Could you check the commit message? It explains it.

To explain a bit more, the code does memset(breakpoints, 0, PM_STRPBRK_CACHE_SIZE); but that's at least theoretically invalid because PM_STRPBRK_CACHE_SIZE=16 and breakpoints = lex_mode.as.list.breakpoints and that's defined as uint8_t breakpoints[11]; or uint8_t breakpoints[7];.
It's an out of bound access, but maybe it doesn't cause any issue in practice due to alignment and/or the union, nevertheless it seems far safer to use uint8_t breakpoints[PM_STRPBRK_CACHE_SIZE]; (and fixes the GCC 9 error).

@eregon
Copy link
Copy Markdown
Member Author

eregon commented Apr 6, 2026

I switched to gcc images and it's quite a bit faster :) 6m49s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants