[PATCH] D29892: ar: add llvm-dlltool support

[Martell Malone via Phabricator via llvm-commits]

martell added inline comments.


================
Comment at: lib/Object/COFFImportFile.cpp:584-585
+    if (E.isWeak()) {
+      std::vector<uint8_t> *WeakBuffer1 = new std::vector<uint8_t>();
+      std::vector<uint8_t> *WeakBuffer2 = new std::vector<uint8_t>();
+      Members.push_back(
----------------
ruiu wrote:
> Please don't use raw `new`s. You really want to avoid doing that, not only in this patch, but in other patches as well. You are leaking memory. I believe you are doing this because otherwise you would be accessing free'd memory, but leaking memory is also a bug which is not acceptable. Using `new` is not a solution for a use-after-free bug.
Sorry about that Rui, I didn't even realise I used `new` here.
This doesn't even need `new` because it is pushed back onto the vector.
Updating the patch now. Also fixed and tested with mingw-w64.


================
Comment at: lib/Object/COFFObjectFile.cpp:232
   if (Symb.isWeakExternal())
-    Result |= SymbolRef::SF_Weak;
+    Result |= SymbolRef::SF_Weak + SymbolRef::SF_Indirect;
 
----------------
This was meant to be `||`.


Repository:
  rL LLVM

https://reviews.llvm.org/D29892