[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