llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.06k stars 11.08k forks source link

[WPD/LTT] Alias to weak_odr alias is created #62439

Open aeubanks opened 1 year ago

aeubanks commented 1 year ago

This is from https://reviews.llvm.org/D145208.

zero_copy_stream.h:

class ZeroCopyOutputStream {
 public:
  virtual void BackUp() = 0;
  virtual bool AllowsAliasing() const { return false; }
};
class StringOutputStream : public ZeroCopyOutputStream {
 public:
  explicit StringOutputStream();
  void BackUp() override {}
};

class CopyingOutputStreamAdaptor : public ZeroCopyOutputStream {
 public:
  explicit CopyingOutputStreamAdaptor(StringOutputStream* copying_stream) {}
  void BackUp() override {}
  bool AllowsAliasing() const override { return true; }
};
class FileOutputStream : public CopyingOutputStreamAdaptor {
 public:
  explicit FileOutputStream(): CopyingOutputStreamAdaptor(nullptr){}

  ~FileOutputStream() {};
};

class EpsCopyOutputStream {
 public:
  EpsCopyOutputStream(ZeroCopyOutputStream* stream): stream_(stream){}
  int* Trim();
  void EnableAliasing();
  ZeroCopyOutputStream* stream_;
};

main.cc:

#include "zero_copy_stream.h"

int main() {
  auto x = new StringOutputStream();
  FileOutputStream output;
  EpsCopyOutputStream stream(&output);
  stream.Trim();
}

impl_lite.cc:

#include "zero_copy_stream.h"

StringOutputStream::StringOutputStream() {}

coded_stream.cc:

#include "zero_copy_stream.h"

void EpsCopyOutputStream::EnableAliasing() {
  stream_->AllowsAliasing();
}

int* EpsCopyOutputStream::Trim() {
  stream_->BackUp();
}
./build/rel/bin/clang++ -fvisibility=hidden -fno-rtti -flto=thin -fwhole-program-vtables -O3 ~/tmp/main.cc ~/tmp/impl_lite.cc ~/tmp/coded_stream.cc -o /dev/null -fuse-ld=lld -Wl,-mllvm,-print-changed

after LowerTypeTests in ld-temp.o, you'll see

@0 = private constant { { [4 x ptr] }, [0 x i8], { [4 x ptr] } } { { [4 x ptr] } { [4 x ptr] [ptr null, ptr null, ptr @_ZN26CopyingOutputStreamAdaptor6BackUpEv, ptr @_ZNK26CopyingOutputStreamAdaptor14AllowsAliasingEv] }, [0 x i8] zeroinitializer, { [4 x ptr] } { [4 x ptr] [ptr null, ptr null, ptr @_ZN18StringOutputStream6BackUpEv, ptr @_ZNK20ZeroCopyOutputStream14AllowsAliasingEv] } }, align 8

@__typeid__ZTS20ZeroCopyOutputStream_8_unique_member = hidden alias i8, getelementptr (i8, ptr @_ZTV16FileOutputStream, i64 16)
@_ZTV16FileOutputStream = weak_odr hidden alias { [4 x ptr] }, ptr @0

I'm still struggling to understand aliases and weak symbols, but as mentioned in https://reviews.llvm.org/D145208 apparently this is an edge case that doesn't have well defined semantics. With some upstream changes, this broke Windows with undefined symbol errors (although this repro is on Linux for ease of debugging, I can give the Windows repro as well if you'd like).

@pcc @teresajohnson

teresajohnson commented 1 year ago

Are you sure this is created by LowerTypeTests and not WPD? I see the typeid...*_unique_member alias being created here by WPD: https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/IPO/WholeProgramDevirt.cpp#L1343

Looks like this kind of alias is only created with the hybrid ThinLTO+Regular LTO WPD that comes with enabling ThinLTO + WPD but not disabling module splitting. Specifically this looks like it is related to unique return value optimization. I'm not as familiar with these optimizations - @pcc can you comment on what semantics are intended here?

Should this instead look like: @typeidZTS20ZeroCopyOutputStream_8_unique_member = hidden alias i8, getelementptr (i8, ptr @0, i64 16)

(i.e. if the GV being used in the aliasee expression is itself a weak alias, use its base object instead)?

aeubanks commented 1 year ago

__typeid__ZTS20ZeroCopyOutputStream_8_unique_member (hidden alias to _ZTV16FileOutputStream) is created in WPD, but LTT changes _ZTV16FileOutputStream from a weak_odr constant to a weak_odr alias to a private constant.

llvmbot commented 9 months ago

@llvm/issue-subscribers-backend-x86

This is from https://reviews.llvm.org/D145208. `zero_copy_stream.h`: ```hpp class ZeroCopyOutputStream { public: virtual void BackUp() = 0; virtual bool AllowsAliasing() const { return false; } }; class StringOutputStream : public ZeroCopyOutputStream { public: explicit StringOutputStream(); void BackUp() override {} }; class CopyingOutputStreamAdaptor : public ZeroCopyOutputStream { public: explicit CopyingOutputStreamAdaptor(StringOutputStream* copying_stream) {} void BackUp() override {} bool AllowsAliasing() const override { return true; } }; class FileOutputStream : public CopyingOutputStreamAdaptor { public: explicit FileOutputStream(): CopyingOutputStreamAdaptor(nullptr){} ~FileOutputStream() {}; }; class EpsCopyOutputStream { public: EpsCopyOutputStream(ZeroCopyOutputStream* stream): stream_(stream){} int* Trim(); void EnableAliasing(); ZeroCopyOutputStream* stream_; }; ``` `main.cc`: ```cpp #include "zero_copy_stream.h" int main() { auto x = new StringOutputStream(); FileOutputStream output; EpsCopyOutputStream stream(&output); stream.Trim(); } ``` `impl_lite.cc`: ```cpp #include "zero_copy_stream.h" StringOutputStream::StringOutputStream() {} ``` `coded_stream.cc`: ```cpp #include "zero_copy_stream.h" void EpsCopyOutputStream::EnableAliasing() { stream_->AllowsAliasing(); } int* EpsCopyOutputStream::Trim() { stream_->BackUp(); } ``` ```shell ./build/rel/bin/clang++ -fvisibility=hidden -fno-rtti -flto=thin -fwhole-program-vtables -O3 ~/tmp/main.cc ~/tmp/impl_lite.cc ~/tmp/coded_stream.cc -o /dev/null -fuse-ld=lld -Wl,-mllvm,-print-changed ``` after LowerTypeTests in `ld-temp.o`, you'll see ```ll @0 = private constant { { [4 x ptr] }, [0 x i8], { [4 x ptr] } } { { [4 x ptr] } { [4 x ptr] [ptr null, ptr null, ptr @_ZN26CopyingOutputStreamAdaptor6BackUpEv, ptr @_ZNK26CopyingOutputStreamAdaptor14AllowsAliasingEv] }, [0 x i8] zeroinitializer, { [4 x ptr] } { [4 x ptr] [ptr null, ptr null, ptr @_ZN18StringOutputStream6BackUpEv, ptr @_ZNK20ZeroCopyOutputStream14AllowsAliasingEv] } }, align 8 @__typeid__ZTS20ZeroCopyOutputStream_8_unique_member = hidden alias i8, getelementptr (i8, ptr @_ZTV16FileOutputStream, i64 16) @_ZTV16FileOutputStream = weak_odr hidden alias { [4 x ptr] }, ptr @0 ``` I'm still struggling to understand aliases and weak symbols, but as mentioned in https://reviews.llvm.org/D145208 apparently this is an edge case that doesn't have well defined semantics. With some upstream changes, this broke Windows with undefined symbol errors (although this repro is on Linux for ease of debugging, I can give the Windows repro as well if you'd like). @pcc @teresajohnson