8cc icon indicating copy to clipboard operation
8cc copied to clipboard

8cc codegen is accessing (potentially) uninitialized struct fields

Open kiwidrew opened this issue 10 years ago • 3 comments

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 a malloc() implementation that does not always clear memory before allocating it to the user program.

The symptom was maybe_emit_bitshift_load() and maybe_emit_bitshift_save() generating bogus SHR and SHL instructions when the bitsize field (signed int32) is less than or equal to zero, which happens frequently when referencing memory which hasn't been cleared. Thankfully GNU as caught the issue and aborted without generating an object file.

To be safe, I've changed all malloc(size) calls to use calloc(nmemb, size) instead.

kiwidrew avatar Jun 25 '15 13:06 kiwidrew

Thanks. I think we should use calloc only when needed. Your patch seems to use calloc even if an allocated memory is initialized immediately.

rui314 avatar Jun 25 '15 16:06 rui314

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.

andrewchambers avatar Jun 26 '15 03:06 andrewchambers

Just to be clear, commit 141d508 is all that is needed to avoid test case failures due to the use-before-initialization error in the Type struct. I added commit a198b17 as extra insurance, since it wasn't entirely obvious that Type was the only struct to suffer from a use-before-initialization error.

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 malloc() with calloc() merely serves to ensure that these assumptions hold true in all cases.

kiwidrew avatar Jun 26 '15 05:06 kiwidrew