[PATCH] D146908: [clang][MinGW] Add asan link flags before other libs and objects

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun added a comment.

In D146908#4226926 <https://reviews.llvm.org/D146908#4226926>, @mstorsjo wrote:

> I tested this, and this does fix the repro from the linked issue.

Thanks for testing!

> I wonder if we do want to move all of these before the app itself, or if it's enough to just move the reference to `asan_dynamic-<arch>.dll.a` earlier, and keep the rest as it was? Especially the whole-archive bit ends up linking a bunch of things statically, ordered way before the app itself. On one hand, I wonder if it would have subtle effects that we don't notice until way later, and we should only change as little as we need (moving the import library reference earlier), or if there are other potential benefits to moving the wholearchive-linked bits earlier too? (I'm wondering about things like constructor execution orders and such.)

That is a valid concern, especially if we want to backport this change to 16.x. I didn't think about this clearly before. With this in mind, I would change the patch to only add a duplicate of `asan_dynamic-<arch>.dll.a` early and keep the existing flags at their old position.

I can do some experiments with the new asan test setup I have and see if there are other benefits of moving the other flags. That will be for separate patches.



================
Comment at: clang/test/Driver/mingw-sanitizers.c:1
-// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-I686 %s
-// ASAN-I686: "{{.*}}libclang_rt.asan_dynamic-i386.dll.a"
-// ASAN-I686: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
-// ASAN-I686: "--require-defined" "___asan_seh_interceptor"
-// ASAN-I686: "--whole-archive" "{{.*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
-
-// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-X86_64 %s
-// ASAN-X86_64: "{{.*}}libclang_rt.asan_dynamic-x86_64.dll.a"
+// RUN: echo -n > %t.a
+// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent %/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%/t.a %s
----------------
mstorsjo wrote:
> Hm, what does this do - some sort of more portable version of just touching a file to create it?
Yeah that's just creating the file (though it would also overwrite the existing content). I believe lit has `echo` built-in but `touch` may not be available on Windows so this seems like a good idea to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146908/new/

https://reviews.llvm.org/D146908