================
@@ -47,114 +138,96 @@ mlir::Value
CIRBasicAliasAnalysis::getUnderlyingObject(mlir::Value val) {
break;
}
- // Pointer stride: only strip through when we can prove the access stays
- // within the bounds of the underlying allocation.
+ // A stride moves the pointer by `stride * sizeof(pointee)` bytes.
if (auto strideOp = mlir::dyn_cast<cir::PtrStrideOp>(defOp)) {
- auto constOp = strideOp.getStride().getDefiningOp<cir::ConstantOp>();
- if (constOp) {
- if (auto intAttr = mlir::dyn_cast<cir::IntAttr>(constOp.getValue())) {
- APInt stride = intAttr.getValue();
-
- // Zero stride is trivially in-bounds.
- if (stride.isZero()) {
- LDBG() << "Walking past zero-strided PtrStrideOp";
- val = strideOp.getBase();
- continue;
- }
- }
- }
- // Dynamic stride or unverifiable bounds — stop here conservatively.
- LDBG() << "Non-zero or dynamic PtrStrideOp, stopping";
- break;
+ LDBG() << "Walking past PtrStrideOp";
+ addToOffset(offset,
+ scaleOffset(getConstantIndex(strideOp.getStride()),
+ getTypeSizeInBytes(strideOp.getElementType(),
+ dataLayout)));
+ val = strideOp.getBase();
+ continue;
}
- // Handle special cases for zero-offset sub-object accesses.
- if (auto op = mlir::dyn_cast<cir::GetMemberOp>(defOp)) {
- if (op.getIndex() == 0) {
- LDBG() << "GetMemberOp[0], following to underlying object";
- val = op.getAddr();
- continue;
- } else {
- LDBG() << "GetMemberOp, non-zero index, stopping";
- break;
- }
+ // A record member sits at a fixed offset given by the record layout.
+ if (auto memberOp = mlir::dyn_cast<cir::GetMemberOp>(defOp)) {
+ LDBG() << "Walking past GetMemberOp";
+ auto recordTy =
+ mlir::cast<cir::RecordType>(memberOp.getAddrTy().getPointee());
+ std::optional<int64_t> memberOffset;
+ if (!recordTy.isIncomplete())
+ memberOffset =
+ recordTy.getElementOffset(dataLayout, memberOp.getIndex());
+ addToOffset(offset, memberOffset);
+ val = memberOp.getAddr();
+ continue;
}
- if (auto op = mlir::dyn_cast<cir::GetElementOp>(defOp)) {
- cir::IntAttr index;
- if (auto constOp = op.getIndex().getDefiningOp<cir::ConstantOp>())
- index = mlir::dyn_cast<cir::IntAttr>(constOp.getValue());
- if (index && index.getValue().isZero()) {
- LDBG() << "GetElementOp[0], following to underlying object";
- val = op.getBase();
- continue;
- }
- LDBG() << "GetElementOp, non-zero or dynamic index, stopping";
- break;
+
+ // An array element sits at `index * sizeof(element)` bytes into the array.
+ if (auto elementOp = mlir::dyn_cast<cir::GetElementOp>(defOp)) {
+ LDBG() << "Walking past GetElementOp";
+ addToOffset(offset,
+ scaleOffset(getConstantIndex(elementOp.getIndex()),
+ getTypeSizeInBytes(elementOp.getElementType(),
+ dataLayout)));
+ val = elementOp.getBase();
+ continue;
}
- if (auto op = mlir::dyn_cast<cir::BaseClassAddrOp>(defOp)) {
- // A zero byte offset means the base subobject starts at the same address
- // as the derived object.
- if (op.getOffset().isZero()) {
- LDBG() << "BaseClassAddrOp[0], following to underlying object";
- val = op.getDerivedAddr();
- continue;
- }
- LDBG() << "BaseClassAddrOp, non-zero offset, stopping";
- break;
+
+ // A base class subobject starts the given number of bytes into the derived
+ // object.
+ if (auto baseOp = mlir::dyn_cast<cir::BaseClassAddrOp>(defOp)) {
+ LDBG() << "Walking past BaseClassAddrOp";
+ addToOffset(offset, baseOp.getOffset().tryZExtValue());
----------------
erichkeane wrote:
> I just assumed there were target architectures that supported valid
> zero-addresses based on the fact that we have handling for it.
Maybe? I DO know we are often pretty pedantic about "but what about mystery
arch that we don't support?!" in many places, but its also entirely/equally
possible I am not aware of one we use. Perhaps some sort of 'bare
metal'/freestanding implementation?
> Perhaps what I intended to do about the case where we aren't assuming
> non-null wasn't clear. I understand that the operation needs it, and it will
> lower to IR that checks for null and returns null with no offset when the
> input is null. However, I wanted to ignore that possibility for purposes of
> alias analysis. My reasoning is that if the input pointer is null (and
> therefore the result is null), any dereference of that null pointer would be
> UB, and so it wouldn't matter if we gave an incorrect alias analysis result
> for that access.
On the face, that seems reasonable.
> To be more explicit. This is what I'm saying:
>
> ```
> void foo() { bar(nullptr); }
> int bar(Derived *p) {
> Base2 *p2 = static_cast<Base2*>(p); // return null
> int *pi = (int*)p; // also null
> pi++; // pi is now 4
> *pi = 1; // dereference 4 --> UB
> return p2->y; // dereferences null --> UB
> }
> ```
That all appears accurate.
> For this case, we would report `MustAlias` for the access at `*pi = 1` and
> the access at `return p2->y`. Even though these are different pointers and
> their accesses wouldn't overlap if they were defined behavior (the address
> zero is valid case), but the `static_cast` would be giving a wrong answer in
> the address zero is valid case, and in the address zero is not valid case,
> both accesses are UB so it doesn't matter that we said they alias.
TBH, if we're doing that `if` chain, we're already breaking the `zero is a
valid pointer` case. The fact that we don't apply the offset means we've
already broken it. However, if you check classic-codegen, it inserts the null
check ONLY based on the expression (see
`CodeGenFunction::ShouldNullCheckClassCastValue`). `ImplicitCastExpr` and
`This` are excluded BECAUSE it is UB to have these expressions be null already.
The other case (`UncheckedDerivedToBase`) is ALSO a case where the Sema has
determined 'null is impossible', not 'we have good reason to allow 0'.
SO I don't see (unless i'm missing something you're seeing?) anywhere we are
acknowledging these '0 is a valid pointer' archs? THAT SAID, if they WERE
supported, we WOULD need a valid value for `null`, and LLVM would have to
support it as a `null` constant. (That is, `0 is a valid address` means you
have to have SOMETHING to be `null`, and `0`!=`null`). So LLVM would have to
do this comparison, and, as a bit of a tautology, `myval eq null-constant`
would ALWAYS be testing for an 'invalid' address.
> The alternative is we return `MayAlias` for any pointer with a
> BaseClassAddrOp that doesn't assume non-null in its pointer use-def chain. So
> for a case like this...
>
> ```
> void foo() { bar(nullptr); }
> int bar(Derived *p) {
> Base2 *p2 = static_cast<Base2*>(p); // return null
> if (p2 == null)
> return -1;
> p->x = 0; // Writes 4 bytes at p, offset zero
> return p2->y; // Reads 4 bytes at p, offset four
> }
> ```
>
> ...we'd return `MayAlias`, even though we could say `NoAlias` if ignored the
> possibility of the static_cast returning null. For this trivial case that
> wouldn't matter, but I'm sure you can see how it could in other cases.
This section/example I'm not really getting? But my understanding of the
alias-analysis is limited.
https://github.com/llvm/llvm-project/pull/219047
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
