lebedev.ri added a comment.

In D67122#1659882 <https://reviews.llvm.org/D67122#1659882>, @rupprecht wrote:

> > Still think this looks good. Have you tried running this on the llvm test 
> > suite, or some other interesting corpus? Would be curious to see any 
> > pre/post patch numbers.
>
> I finally had time this morning to patch this in and give it a shot. (Sorry 
> for the delay... it's been a real busy week :( )




> First, starting off with the good news: I reverted all the fixes I made, and 
> now all the tests fail when running w/ ubsan. Yay!
>  The error we see in each case is `UndefinedBehaviorSanitizer: 
> nullptr-with-nonzero-offset` with the logs containing `runtime error: 
> applying non-zero offset <non-zero> to null pointer`. Which catches the two 
> places where we were adding some non-zero offset to nullptr,

That is good :)

> but doesn't seem to catch the nullptr-after-nonzero-offset case in 
> https://github.com/google/filament/pull/1566 -- instead, it fails later, when 
> the pointer with a value of nullptr is incremented. (Or... maybe this is 
> actually a separate bug. Hmm. Needs some more testing...)

Well yeah, there are quite a few cases in clang codegen where we create `gep 
inbounds` without sanitization, so it may or may not be one of those cases.

> At any rate, I have some more tests to run to get some idea of what % of code 
> this would flag as being bad.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to