Skip to content

Conversation

@cbalint13
Copy link
Contributor

@cbalint13 cbalint13 commented Oct 17, 2023

Fix compilation warnings for clang.


The warnings was:

BUILD/tvm/src/target/llvm/llvm_instance.cc:84:81: warning: reference to stack memory associated with parameter 'Obj' returned [-Wreturn-stack-address]
   84 |   friend ArrayRef<SubtargetFeatureKV>& featViewer(MCSubtargetInfo Obj) { return Obj.*Member; }
      |                                                                                 ^~~
BUILD/tvm/src/target/llvm/llvm_instance.cc:833:49: 
 warning: cast from 'const llvm::MCSubtargetInfo *' to 'llvm::MCSubtargetInfo *' drops const qualifier [-Wcast-qual]
  833 |       llvm::featViewer(*(llvm::MCSubtargetInfo*)MCInfo);
      |                                                 ^
BUILD/tvm/src/target/llvm/llvm_instance.cc:84:81: 
 warning: reference to stack memory associated with parameter 'Obj' returned [-Wreturn-stack-address]
   84 |   friend ArrayRef<SubtargetFeatureKV>& featViewer(MCSubtargetInfo Obj) { return Obj.*Member; }
      |                                                                                 ^~~
BUILD/tvm/src/target/llvm/llvm_instance.cc:833:13: note: in instantiation of member function 'llvm::featViewer' requested here
  833 |       llvm::featViewer(*(llvm::MCSubtargetInfo*)MCInfo);
      |           
$ rpm -q clang
clang-17.0.2-1.fc40.x86_64

No warnings was for: gcc (13.2) or vscode (2019) sides.

Cc: @junrushao

Copy link
Contributor

@quic-sanirudh quic-sanirudh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks.

@junrushao
Copy link
Member

Hey @cbalint13 thanks a lot for the swift fix!! Yeah this issue was reported by clang++ on macOS, and I can confirm it’s gone with this change. Thanks again 🙏

@cbalint13
Copy link
Contributor Author

Hey @cbalint13 thanks a lot for the swift fix!! Yeah this issue was reported by clang++ on macOS, and I can confirm it’s gone with this change. Thanks again 🙏

With pleasure, anytime.


Just some extra info:

I also checked using gcc's "hardening" stack-protector flags, being a default thing for fedora/redhat releng, and it is fine.

Short, the very flag: -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong

The full flags, added by rhel/fedora at packaging creation time:

$ rpm --eval %{optflags}
-O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall 
-Wno-complain-wrong-lang -Werror=format-security -Werror=implicit-function-declaration 
-Werror=implicit-int -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS 
-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong 
-specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64   -mtune=generic 
-fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer 
-mno-omit-leaf-frame-pointer

@junrushao
Copy link
Member

Thanks for testing with the extra stack protector 🙏❤️ Let's get it merged soon!

@quic-sanirudh quic-sanirudh merged commit 8048341 into apache:main Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants