cat > test.cc <<EOF #include <assert.h> #include <stdint.h> #define TRIVIAL_ABI __attribute__((trivial_abi)) template<class T> class TRIVIAL_ABI trivial_offset_ptr { intptr_t value_; public: trivial_offset_ptr(T *p) : value_((const char*)p - (const char*)this) {} trivial_offset_ptr(const trivial_offset_ptr& rhs) : value_((const char*)rhs.get() - (const char*)this) {} T *get() const { return (T *)((const char *)this + value_); } trivial_offset_ptr& operator=(const trivial_offset_ptr& rhs) { value_ = ((const char*)rhs.get() - (const char*)this); return *this; } trivial_offset_ptr& operator+=(int diff) { value_ += (diff * sizeof (T)); return *this; } }; trivial_offset_ptr<int> incr(trivial_offset_ptr<int> p) { p += 1; return p; } int main() { int a[10]; trivial_offset_ptr<int> op = &a[4]; trivial_offset_ptr<int> top = &a[4]; top = incr(top); assert(top.get() == &a[5]); } EOF clang++ -std=c++17 -O1 test.cc -o a.out ./a.out # works great! clang++ -std=c++17 -O2 test.cc -o a.out ./a.out Assertion failed: (top.get() == &a[5]), function main, file test.cc, line 34. Abort trap: 6 ==== This is super sneaky UB-ful code in theory, I'm sure, but IMHO Clang is still doing something not-conservative-enough here. Especially, notice that if you remove the unused local variable `op` from `main`, the assertion will pass. Put the unused variable back, and the assertion will fail. Documentation for [[trivial_abi]] is here. It does not mention any caveats about "don't implement offset_ptr with this attribute." I would grudgingly accept that outcome if it were proposed (but I'd rather the code Just Work). https://clang.llvm.org/docs/AttributeReference.html#trivial-abi-clang-trivial-abi P.S. I am aware that [[trivial_abi]] is super new, and I love it, and I don't mean to be harsh at all in this first corner-case bug filed against it. :)
$ ~/llvm/build/bin/clang++ -std=c++17 test.cc -O2 -v clang version 7.0.0 (git@github.com:llvm-mirror/clang.git 4e9568799bf3497f82dca7771dc870324f42bcc9) (https://git.llvm.org/git/llvm.git fbad418d179daabb0acb0bac3df21482d9ff35cf) Target: x86_64-apple-darwin17.5.0 Thread model: posix InstalledDir: /Users/aodwyer/llvm/build/bin "/Users/aodwyer/llvm/build/bin/clang-7" -cc1 -triple x86_64-apple-macosx10.13.0 -Wdeprecated-objc-isa-usage -Werror=deprecated-objc-isa-usage -emit-obj -disable-free -main-file-name test.cc -mrelocation-model pic -pic-level 2 -mthread-model posix -mdisable-fp-elim -masm-verbose -munwind-tables -target-cpu penryn -dwarf-column-info -debugger-tuning=lldb -target-linker-version 305 -v -resource-dir /Users/aodwyer/llvm/build/lib/clang/7.0.0 -stdlib=libc++ -O2 -std=c++17 -fdeprecated-macro -fdebug-compilation-dir /tmp -ferror-limit 19 -fmessage-length 153 -stack-protector 1 -fblocks -fencode-extended-block-signature -fregister-global-dtors-with-atexit -fobjc-runtime=macosx-10.13.0 -fcxx-exceptions -fexceptions -fmax-type-align=16 -fdiagnostics-show-option -fcolor-diagnostics -vectorize-loops -vectorize-slp -o /var/folders/0l/9t0yv2890_g4wgmy53n_mg7w0000gy/T/test-9d1fd1.o -x c++ test.cc clang -cc1 version 7.0.0 based upon LLVM 7.0.0svn default target x86_64-apple-darwin17.5.0 ignoring nonexistent directory "/usr/include/c++/v1" #include "..." search starts here: #include <...> search starts here: /Users/aodwyer/llvm/build/include/c++/v1 /usr/local/include /Users/aodwyer/llvm/build/lib/clang/7.0.0/include /usr/include /System/Library/Frameworks (framework directory) /Library/Frameworks (framework directory) End of search list. "/usr/bin/ld" -demangle -lto_library /Users/aodwyer/llvm/build/lib/libLTO.dylib -dynamic -arch x86_64 -macosx_version_min 10.13.0 -o a.out /var/folders/0l/9t0yv2890_g4wgmy53n_mg7w0000gy/T/test-9d1fd1.o -lc++ -lSystem
The definition of trivial_abi is that when you pass the class to a function, it gets memcpy'ed. So your code to adjust the offset doesn't get called consistently. This would be obvious at -O0 if you tried to call get() inside the implementation of incr(). Your testcase "works" at lower optimization levels due to pure luck, I think: the offsets of the two skipped copy constructor calls cancel each other out, or something like that.
We should update the documentation to be a bit clearer about the implications of the fact that we will "pass and return the type using the C ABI for the underlying type" (in particular, how it will break types that contain pointers to themselves). But this is not a miscompile.
Okay, I can buy that. To clarify: If I make a copy, it should still call the copy constructor exactly once; if I move, it should still call the move constructor exactly once; but it might *also* memcpy the bits to a new address either before or after the copy/move. If my code relies on its *not* doing that extra memcpy, then I shouldn't use [[trivial_abi]]. I wish there were a way to detect this situation programmatically, but I can totally believe that that's impossible. Improving the docs to include something like the above clarification sounds to me like an appropriate resolution.
(In reply to Arthur O'Dwyer from comment #4) > To clarify: If I make a copy, it should still call the copy constructor > exactly once; if I move, it should still call the move constructor exactly > once; Copies and moves are still subject to elision, but other than that, yes. (And in particular, you still get one constructor call for each destructor call.) > but it might *also* memcpy the bits to a new address either before or > after the copy/move. If my code relies on its *not* doing that extra memcpy, > then I shouldn't use [[trivial_abi]]. Yes. You can think of the behavior being as if the parameter object is first constructed, and then relocated from the caller's preferred location into the callee's preferred location. And then the destructor runs in the callee (because it must, as the caller has no access to the object any more), potentially in the wrong order relative to other function parameters.