erichkeane added inline comments.

================
Comment at: clang/lib/AST/ExprConstant.cpp:13540
         ED->getValueRange(Max, Min);
+        --Max;
 
----------------
shafik wrote:
> erichkeane wrote:
> > I don't think this is the correct answer.  Even though the other use of 
> > this seems to 'work', `getValueRange` is still returning wrapped values 
> > here.  The fix is to figure out how to fix the math in getValueRange, and 
> > change the sanitizer's IR generation if necessary.
> So the other user of these values is `getRangeForLoadFromType(...)` through 
> `getRangeForType(...)` which is used to generate the [range 
> metadata](https://llvm.org/docs/LangRef.html#range-metadata) and in this case 
> the range is exclusive on the right side:
> 
> > The pair a,b represents the range [a,b).
> 
> and it is allowed to wrap:
> 
> > The range is allowed to wrap.
> 
> So I believe the behavior will be correct. 
> 
> Even though the range looks off in this example: 
> https://godbolt.org/z/z7d9PKoMn
> 
> ```
> !{i32 0, i32 -2147483648}
> ```
> 
> I think it does the right thing.
> 
> Note the language ref also says:
> 
> > The type must match the type loaded by the instruction.
> 
> and a quick experiment to use `i33` confirms this breaks.
Hmm... ok.  Well, its quite unfortunate that 2 of our 3 uses of this are having 
to hack around this behavior a bit, but I guess this will have to be acceptable 
short-term then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130811

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

Reply via email to