search

rui314 / chibicc

A small C compiler
MIT License
9.57k stars 871 forks source link

asan issues with `memcp`, use `strncmp` instead? #48

Open math4tots opened 3 years ago

math4tots commented 3 years ago

I was just testing out chibicc on a mac with

clang -g \
  -fsanitize=address \
  -fsanitize=undefined \
  -Wno-switch -Wno-format *.c && \
./a.out -I<dummy-headers-include-path> *.c -S

and the address sanitizer complains about this line:

https://github.com/rui314/chibicc/blob/90d1f7f199cc55b13c7fdb5839d1409806633fdb/tokenize.c#L80

I'm guessing that this is probably because tok->len can sometimes be longer than the string stored in op, so memcmp can sometimes access memory out of bounds.

Using strncmp here instead of memcmp seems to fix the issue.

I would open a pull request, but I read your README, and figured you probably want to make the change yourself :p

On the other hand, I don't think this would ever actually be an issue at runtime, so if you don't think this issue is worth the hassle of rebasing, please feel free to just close and ignore this issue :)

best,

rui314 commented 3 years ago

Thank you for pointing that out. I think we should use strncmp as you mentioned. I'll keep this bug open until I make a fix to my local repo. Thanks!