Suppress new profiler_builtins warning since LLVM 11

[richkadel]

GCC is emitting a new warning while building profiler_builtins with gcc. This should suppress that warning.

r? @cuviper

[richkadel]

@tmandry FYI

[richkadel]

@cuviper - I updated the PR description to reflect the change made as recommended by @tmiasko .

This is now basically a one-liner.

PTAL... Thanks!

[cuviper]

It looks like that warning flag was fairly recently added in this commit (gcc-9.1.0), so it's definitely better conditionally.

However, I think this should probably be fixed in LLVM:

https://reviews.llvm.org/D82253
https://github.com/rust-lang/llvm-project/blob/45790d79496be37fbce6ec57abad5af8fa7a34d7/compiler-rt/lib/profile/GCDAProfiling.c#L631-L640

#ifndef _WIN32
// __attribute__((destructor)) and destructors whose priorities are greater than
// 100 run before this function and can thus be tracked. The priority is
// compatible with GCC 7 onwards.
__attribute__((destructor(100)))
#endif
static void llvm_writeout_and_clear(void) {
  llvm_writeout_files();
  fn_list_remove(&writeout_fn_list);
}

It sounds like the intent was to avoid this warning, but I think it would actually need to start at 101. Digging in GCC history, MAX_RESERVED_INIT_PRIORITY has always been 100, and the warning was added here (gcc-4.3.0) for <= MAX. I'm not sure where GCC 7 comes in though...

cc @MaskRay

[cuviper]

For more context, here is how the warning is documented in the manpage as of GCC 10.2:

   -Wno-prio-ctor-dtor                                                    
       Do not warn if a priority from 0 to 100 is used for constructor or 
       destructor.  The use of constructor and destructor attributes allow
       you to assign a priority to the constructor/destructor to control  
       its order of execution before "main" is called or after it returns.
       The priority values must be greater than 100 as the compiler       
       reserves priority values between 0--100 for the implementation.    

[richkadel]

@cuviper - I agree there should be a fix in LLVM, but until then, Rust builds (with profiler=true) are not clean. This is creating multiple occurrences of yellow warning messages during builds. (Very distracting, and unnecessary.) Most other warnings are suppressed in Rust builds.

This is a stopgap solution, not knowing how long it will take to resolve upstream.

Are you OK with this?

[cuviper]

I have also filed https://bugs.llvm.org/show_bug.cgi?id=47399, so let's give that a chance for response. If it's really as simple as raising this priority to 101, we can patch that rather than papering over it.

[MaskRay]

It sounds like the intent was to avoid this warning, but I think it would actually need to start at 101. Digging in GCC history, MAX_RESERVED_INIT_PRIORITY has always been 100, and the warning was added here (gcc-4.3.0) for <= MAX. I'm not sure where GCC 7 comes in though...

No. __attribute__((desctructor(100))) is correct. The intention is that .fini_array.00101 can be recorded while .fini_array.00100 and .fini_array.00099 can't.

The patch can suppress GCC>=9 -Wprio-ctor-dtor warning but GCC 8 still warns

--- i/compiler-rt/lib/profile/GCDAProfiling.c
+++ w/compiler-rt/lib/profile/GCDAProfiling.c
@@ -628,6 +628,9 @@ void llvm_writeout_files(void) {
   }
 }
 
+#if defined(__GNUC__) && __GNUC__ >= 9
+#pragma GCC diagnostic ignored "-Wprio-ctor-dtor"
+#endif
 #ifndef _WIN32
 // __attribute__((destructor)) and destructors whose priorities are greater than
 // 100 run before this function and can thus be tracked. The priority is

[cuviper]

Well, pragma-masking this function on GCC 9 is an improvement, more focused than Rust doing it from the command line. @MaskRay are you going to make this change on the LLVM side?

[MaskRay]

Well, pragma-masking this function on GCC 9 is an improvement, more focused than Rust doing it from the command line. @MaskRay are you going to make this change on the LLVM side?

Yes. Suppressed GCC 9 -Wprio-ctor-dtor in llvm/llvm-project@1cfde14

[richkadel]

@cuviper will you be updating the Rust fork with this fix?

[cuviper]

@richkadel In the LLVM bug, I've asked for the patch to be backported to the 11.x branch. If you'd like to see it sooner, you can follow our backport procedure and I'll approve those PRs.
https://rustc-dev-guide.rust-lang.org/backend/updating-llvm.html#bugfix-updates

[richkadel]

This PR is no longer needed as of:

rust-lang/llvm-project#73 and #76341