This is an archive of the discontinued LLVM Phabricator instance.

[CodeGenCXX] annotate a GEP to a derived class with 'inbounds' (PR35909)
ClosedPublic

Authored by spatel on Jan 18 2018, 10:05 AM.

Details

Summary

I'm not sure if the code comment is adequate or even correct, but hopefully the change itself is valid.

Eli cited this section of the standard in PR35909 ( https://bugs.llvm.org/show_bug.cgi?id=35909 ):
[expr.static.cast] p11: "If the prvalue of type “pointer to cv1 B” points to a B that is actually a subobject of an object of type D, the resulting pointer points to the enclosing object of type D. Otherwise, the behavior is undefined."

In the motivating case in the bug report, LLVM can't eliminate a nullptr check because a GEP is not marked with 'inbounds':

class A {
    int a;
};
class B {
    int b;
public:
    A *getAsA();
};
class X : public A, public B {
    int x;
};

A *B::getAsA() {
    if (b == 42) {
        auto temp = static_cast<X*>(this);
        //__builtin_assume(temp != nullptr);
        return temp;
    }
    return nullptr;
}

void helper(A *);

void test(B *b) {
    auto temp = b->getAsA();
    if (temp)
        return helper(temp);
}

Diff Detail

Event Timeline

spatel created this revision.Jan 18 2018, 10:05 AM
efriedma added inline comments.Jan 18 2018, 11:24 AM
lib/CodeGen/CGClass.cpp
410

Not sure this comment really adds anything, unless you want to cite the standard.

test/CodeGenCXX/catch-undef-behavior.cpp
391

We probably want a test which checks the output when ubsan isn't enabled.

spatel updated this revision to Diff 130488.Jan 18 2018, 1:37 PM

Patch updated:

  1. Removed comment that didn't add value.
  2. Added test with no sanitizing, reduced from PR35909.
This revision is now accepted and ready to land.Jan 18 2018, 3:31 PM
This revision was automatically updated to reflect the committed changes.