[llvm] [llvm] Fix __builtin_object_size interaction between Negative Offset … (PR #111827)
Harald van Dijk via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 10 07:18:52 PDT 2024
================
@@ -981,18 +992,39 @@ ObjectSizeOffsetVisitor::combineSizeOffset(SizeOffsetAPInt LHS,
SizeOffsetAPInt RHS) {
if (!LHS.bothKnown() || !RHS.bothKnown())
return ObjectSizeOffsetVisitor::unknown();
-
- switch (Options.EvalMode) {
- case ObjectSizeOpts::Mode::Min:
- return (getSizeWithOverflow(LHS).slt(getSizeWithOverflow(RHS))) ? LHS : RHS;
- case ObjectSizeOpts::Mode::Max:
- return (getSizeWithOverflow(LHS).sgt(getSizeWithOverflow(RHS))) ? LHS : RHS;
- case ObjectSizeOpts::Mode::ExactSizeFromOffset:
- return (getSizeWithOverflow(LHS).eq(getSizeWithOverflow(RHS)))
- ? LHS
- : ObjectSizeOffsetVisitor::unknown();
- case ObjectSizeOpts::Mode::ExactUnderlyingSizeAndOffset:
- return LHS == RHS ? LHS : ObjectSizeOffsetVisitor::unknown();
+ // If the ConstantOffset we add in the end is negative, then we're actually
+ // interested in selecting the nodes based on their offset rather than their
+ // size.
----------------
hvdijk wrote:
Are we? I cannot really understand how this logic works, sorry. When `Options.EvalMode==ObjectSizeOpts::Mode::ExactSizeFromOffset`, that specifies that we should combine `LHS` and `RHS` when the remaining sizes of `LHS` and `RHS` are the same, that is how it is documented, and it feels risky to add a field to `ObjectSizeOffsetVisitor` that makes it so that this function no longer behaves as documented.
https://github.com/llvm/llvm-project/pull/111827
More information about the llvm-commits
mailing list