8cc codegen is accessing (potentially) uninitialized struct fields #67
Conversation
typically initialized; these fields are only assigned values for actual
bitfield members. However, the 'maybe_emit_bitshift_load' and
'maybe_emit_bitshift_save' functions (called by 'emit_[gl]{load,save}'
functions) access these fields unconditionally.
Simple fix is to replace malloc() with calloc(), which ensures that
the memory region has been cleared.
(This bug was discovered while working to port 8cc to OpenBSD, which
provides a malloc() implementation that does not necessarily clear the
memory region upon allocation.)
|
Thanks. I think we should use calloc only when needed. Your patch seems to use calloc even if an allocated memory is initialized immediately. |
|
It is handy to have a sensible default for most values. If this was going to be added, It would be better style to create a wrapper around malloc which checks for errors and does the zeroing. |
|
Just to be clear, commit It would seem that the code (mostly) follows the standard C language convention where each struct member's "default" or uninitialized value is assumed to be zero. Replacing |
When using a
malloc()implementation that doesn't clear the memory region (which is permitted behaviour according to the standards) it is quite easy for 8cc to generate invalid assembly code. I first noticed this issue when compiling 8cc under OpenBSD, which has amalloc()implementation that does not always clear memory before allocating it to the user program.The symptom was
maybe_emit_bitshift_load()andmaybe_emit_bitshift_save()generating bogus SHR and SHL instructions when thebitsizefield (signed int32) is less than or equal to zero, which happens frequently when referencing memory which hasn't been cleared. Thankfully GNUascaught the issue and aborted without generating an object file.To be safe, I've changed all
malloc(size)calls to usecalloc(nmemb, size)instead.