[PATCH] D39017: [CMake] Build Fuchsia toolchain as -O3

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

LGTM
Since we use lld, -Wl,-O2 also helps make the binaries smaller.


Repository:
  rL LLVM

https://reviews.llvm.org/D39017



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39176: [Driver] Use ld.lld directly for Fuchsia rather than passing flavor

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

This seems like how it always should have been, since -fuse-ld=lld sets it to 
ld.lld, not lld.
Looks to me like the existing MipsLinux and WebAssembly implementations are 
wrong too and only BareMetal is right.


Repository:
  rL LLVM

https://reviews.llvm.org/D39176



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39270: [CMake] Include clang-refactor in Fuchsia toolchain

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rL LLVM

https://reviews.llvm.org/D39270



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39273: [CMake] Build host builtins in Fuchsia toolchain even on Darwin

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rL LLVM

https://reviews.llvm.org/D39273



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51573: [Driver] Search LibraryPaths when handling -print-file-name

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

The log message could give concrete examples.




Comment at: clang/lib/Driver/Driver.cpp:4169
 std::string Driver::GetFilePath(StringRef Name, const ToolChain &TC) const {
-  // Respect a limited subset of the '-Bprefix' functionality in GCC by
-  // attempting to use this prefix when looking for file paths.
-  for (const std::string &Dir : PrefixDirs) {
-if (Dir.empty())
-  continue;
-SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
-llvm::sys::path::append(P, Name);
-if (llvm::sys::fs::exists(Twine(P)))
-  return P.str();
-  }
+  auto FindPath = [&](const llvm::SmallVectorImpl &P)
+  -> llvm::Optional {

A one-line comment before the lambda would help: `// Check for Name in Path.` 
(rename the param to be clearer)



Comment at: clang/lib/Driver/Driver.cpp:4173
+// attempting to use this prefix when looking for file paths.
+for (const std::string &Dir : P) {
+  if (Dir.empty())

I'd always use `const auto&` in a range-for like this, but I guess this was 
just moved around and maybe LLVM style frowns on auto?



Comment at: clang/lib/Driver/Driver.cpp:4179
+  if (llvm::sys::fs::exists(Twine(P)))
+return std::string(P.str());
+}

Can this be `return {P.str()};`?



Comment at: clang/lib/Driver/Driver.cpp:4184
+
+  if (Optional D = FindPath(PrefixDirs))
+return *D;

I'd always use auto in these situations (here and below).



Comment at: clang/lib/Driver/Driver.cpp:4187
 
   SmallString<128> R(ResourceDir);
   llvm::sys::path::append(R, Name);

You could fold these cases into the lambda too by having it take a bool 
UseSysRoot and passing `{ResourceDir}, false`.
Then the main body is a uniform and concise list of paths to check.


Repository:
  rC Clang

https://reviews.llvm.org/D51573



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: lib/Parse/ParseDecl.cpp:244-252
+// If this was declared in a macro, attatch the macro IdentifierInfo to the
+// parsed attribute.
+for (const auto &MacroPair : PP.getAttributeMacros()) {
+  if (SourceLocInSourceRange(AttrTok.getLocation(), MacroPair.first,
+ PP.getSourceManager())) {
+ApplyMacroIIToParsedAttrs(attrs, NumParsedAttrs, MacroPair.second);
+break;

rsmith wrote:
> leonardchan wrote:
> > rsmith wrote:
> > > You shouldn't do this if `NumParsedAttrs != 1`. We're only looking for a 
> > > macro that exactly covers one attribute.
> > > 
> > > (Also, your computation of the number of attributes in the attribute list 
> > > is not correct in the presence of late-parsed attributes.)
> > One of the things we would like is for this to cover more than one 
> > attribute in the attribute list since in sparse, `address_space` is 
> > sometimes used with `noderef`.
> > 
> > So given `# define __user __attribute__((noderef, address_space(1)))`, 
> > `__user` would be saved into the `ParsedAttr` made for `noderef` and 
> > `address_space`.
> > 
> > What would be the appropriate way to track newly added attributes into the 
> > `ParsedAttributes`, including late-parsed attributes?
> > One of the things we would like is for this to cover more than one 
> > attribute in the attribute list since in sparse, `address_space` is 
> > sometimes used with `noderef`.
> 
> Hold on, this is a new requirement compared to what we'd previously discussed 
> (giving a name to an address space). How important is this use case to you?
> 
> I don't think it's a reasonable AST model to assign a macro identifier to an 
> `AttributedType` if the macro defines multiple attributes. If you really want 
> to handle that use case, I think that an identifier on the `AttributedType` 
> is the wrong way to model it, and we should instead be creating a new type 
> sugar node representing a type decoration written as a macro.
> 
> Assuming you want to go ahead with the current patch direction (at least in 
> the short term), please add the "only one attribute in the macro" check.
A single macro that uses multiple attributes is the central use case for us.
It would be fine if it's constrained to a single __attribute__ clause (or 
[[...]] clause) with the attributes comma-separated, as in __attribute__((foo, 
bar)) as opposed to two separate __attribute__((foo)) __attribute__((bar)) in 
the macro, if that matters.  It's even fine if it's constrained to the macro 
being nothing but the __attribute__((foo, bar)) clause (aside from whitespace 
and comments).

Leo can decide how he wants to proceed as far as incremental implementation.
But we won't have a real-world use for the feature only covering a single 
attribute.
We'll start using when it can cover the cases like the Linux __user and __iomem 
examples (address_space + noderef).


Repository:
  rC Clang

https://reviews.llvm.org/D51329



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

I think this wants to be a hard error rather than a warning.  Though since we 
use -Werror anyway if others feel strongly contrary I won't object.


Repository:
  rC Clang

https://reviews.llvm.org/D49511



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41471: [CMake][Fuchsia] Enable assertions

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added a reviewer: phosek.
Herald added a subscriber: mgorny.

Enable assertions in both stages.
Release+Asserts is fast enough.
No need to let insanity through.


Repository:
  rC Clang

https://reviews.llvm.org/D41471

Files:
  cmake/caches/Fuchsia-stage2.cmake
  cmake/caches/Fuchsia.cmake


Index: cmake/caches/Fuchsia.cmake
===
--- cmake/caches/Fuchsia.cmake
+++ cmake/caches/Fuchsia.cmake
@@ -13,6 +13,7 @@
 set(LLVM_ENABLE_ZLIB OFF CACHE BOOL "")
 set(CLANG_PLUGIN_SUPPORT OFF CACHE BOOL "")
 
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
 set(CMAKE_BUILD_TYPE Release CACHE STRING "")
 
 set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -23,6 +23,7 @@
   set(LLDB_CODESIGN_IDENTITY "" CACHE STRING "")
 endif()
 
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
 set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
 set(CMAKE_C_FLAGS_RELWITHDEBINFO "-O3 -gline-tables-only -DNDEBUG" CACHE 
STRING "")
 set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "-O3 -gline-tables-only -DNDEBUG" CACHE 
STRING "")


Index: cmake/caches/Fuchsia.cmake
===
--- cmake/caches/Fuchsia.cmake
+++ cmake/caches/Fuchsia.cmake
@@ -13,6 +13,7 @@
 set(LLVM_ENABLE_ZLIB OFF CACHE BOOL "")
 set(CLANG_PLUGIN_SUPPORT OFF CACHE BOOL "")
 
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
 set(CMAKE_BUILD_TYPE Release CACHE STRING "")
 
 set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -23,6 +23,7 @@
   set(LLDB_CODESIGN_IDENTITY "" CACHE STRING "")
 endif()
 
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
 set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
 set(CMAKE_C_FLAGS_RELWITHDEBINFO "-O3 -gline-tables-only -DNDEBUG" CACHE STRING "")
 set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "-O3 -gline-tables-only -DNDEBUG" CACHE STRING "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43992: [Frontend] Avoid including default system header paths on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D43992



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44605: [Driver] Default to DWARF 5 for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

We aren't actually using DWARF 5 yet AFAICT.  Our builds don't pass -gdwarf-5.  
So I'm not sure we have yet verified that all the DWARF-consuming tools people 
are using with Fuchsia binaries can handle all of DWARF 5 (which has several 
major format changes).  I'd certainly like 5 to be the default, but I think we 
need to establish a set of consumers we care about and verify their format 
version support before we can be sure about this.


Repository:
  rC Clang

https://reviews.llvm.org/D44605



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47668: [Driver][Fuchsia] Pass LTO flags to linker

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D47668



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47758: [Fuchsia] Include install-distribution-stripped in bootstrap targets

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D47758



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48208: [Fuchsia] Enable static libc++, libc++abi, libunwind

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D48208



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53487: [Driver] Support sanitized libraries on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:566
+  for (const auto &LibPath : TC.getLibraryPaths()) {
+if(LibPath.length() > 0) {
+  SmallString<128> P(LibPath);

`!LibPath.empty()`? 


Repository:
  rC Clang

https://reviews.llvm.org/D53487



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:125
!Args.hasArg(options::OPT_static);
+CmdArgs.push_back("-push-state");
+CmdArgs.push_back("-as-needed");

I'd use the `--` version of all these GNU-compatible options since that's the 
GNU-compatible syntax.
(But `-static` is still single-dash.)



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:132
   }
   CmdArgs.push_back("-lm");
 }

`-lm` belongs inside the `--as-needed` umbrella too.


Repository:
  rC Clang

https://reviews.llvm.org/D53854



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56652: [CMake][Fuchsia] Synchronize first and second stage builds

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

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

https://reviews.llvm.org/D56652



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56972: [CMake][Fuchsia] Drop -DNDEBUG, re-enable modules

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

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

https://reviews.llvm.org/D56972



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53787: [Sema] Use proper visibility for global new and delete declarations

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

In https://reviews.llvm.org/D53787#1279899, @rsmith wrote:

> Replacing the global new and delete is supposed to be a whole-program 
> operation (you only get one global allocator). Otherwise you couldn't 
> allocate memory in one DSO and deallocate it in another. (And nor could 
> inline functions etc.)
>
> If you really want to do this weird thing, and you understand what you're 
> getting yourself into, I don't see a problem with having a dedicated flag for 
> it, but don't break all existing users of -fvisibility=.


I don't really understand how these functions are different from other 
functions.  The language standards don't have anything to say about ELF 
visibility.  What you say about "whole-program operation" is true of any global 
symbol.  When we use visibility switches or annotations it's because we want to 
change how global symbols behave.  I don't understand the rationale for 
treating these particular functions differently from all other functions.  It 
is especially bizarre to me that explicit attributes on the definition sites 
are silently ignored for these functions and no others.

Few if any existing users of -fvisibility or visibility attributes use them on 
definitions of operator new and operator delete.  The notion that existing 
users are expecting this bizarrely inconsistent behavior seems pretty 
questionable to me.

But indeed I do know what I'm doing and I am willing to tell the compiler even 
more explicitly if you insist that I should have to do that for some reason.
I don't care what the switch is called.  This is only ever going to be used in 
basically one place in the world (the libc++ definitions when building it for 
hermetic static linking).


Repository:
  rC Clang

https://reviews.llvm.org/D53787



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53787: [Sema] Use proper visibility for global new and delete declarations

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

In https://reviews.llvm.org/D53787#1282975, @rsmith wrote:

> These symbols really are special. Other symbols are introduced explicitly by 
> a declaration, whereas these are declared implicitly by the compiler.


The implicit declaration is the only difference that actually makes sense to me.

> Other symbols must have exactly one definition (modulo the permission for 
> duplicate identical definitions for some cases), but these ones have a 
> default definition that is designed to be overridable by a different 
> definition appearing anywhere in the program.

I don't understand this claim.  These are symbols like any others at link time. 
 A single definition must be supplied as for any other function.

> Other symbols are generally provided in one library and consumed by users of 
> that library, whereas these symbols are typically provided by the main binary 
> and consumed by the libraries that it uses. And so on.

I don't understand this claim.  These symbols are normally defined in libc++.so 
and nowhere else.

>> It is especially bizarre to me that explicit attributes on the definition 
>> sites are silently ignored for these functions and no others.
> 
> Do you have a testcase? That's not the behavior I'm seeing. What I see is 
> that we get a hard error for an explicit attribute on the definition site, 
> because the prior compiler-generated declaration has default visibility. Eg:

You're right.  It's been quite a while since I was fighting with this 
originally.  It might have been GCC that ignored explicit attributes.


Repository:
  rC Clang

https://reviews.llvm.org/D53787



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53787: [Sema] Provide -fvisibility-global-new-delete-hidden option

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

In https://reviews.llvm.org/D53787#1283051, @rsmith wrote:

> In https://reviews.llvm.org/D53787#1282995, @mcgrathr wrote:
>
> > In https://reviews.llvm.org/D53787#1282975, @rsmith wrote:
> >
> > > Other symbols must have exactly one definition (modulo the permission for 
> > > duplicate identical definitions for some cases), but these ones have a 
> > > default definition that is designed to be overridable by a different 
> > > definition appearing anywhere in the program.
> >
> >
> > I don't understand this claim.  These are symbols like any others at link 
> > time.  A single definition must be supplied as for any other function.
>
>
> The program has two choices: either it provides a definition, and that gets 
> used everywhere, or it does not, and the default version (provided by the 
> toolchain) gets used everywhere.


That's true of all global symbols normally provided by libraries.

> This is what the language model requires, and it's so important that the 
> entire program agrees on what the default heap is that we intentionally make 
> this work even when using `-fvisibility=hidden`.

So you're concerned with people who use `-fvisibility=hidden` on the TUs that 
define some functions, and then use dynamic linking with the expectation that 
those definitions will be seen across shared library boundaries.
I don't see how people specifying visibility different than what they actually 
want is different for these particular functions than for any others.  But I'm 
biased towards users who actually understand what they asked the compiler to do.

>>> Other symbols are generally provided in one library and consumed by users 
>>> of that library, whereas these symbols are typically provided by the main 
>>> binary and consumed by the libraries that it uses. And so on.
>> 
>> I don't understand this claim.  These symbols are normally defined in 
>> libc++.so and nowhere else.
> 
> That's not accurate. As noted above, programs can provide their own 
> definitions, which are required to replace the version that would otherwise 
> be implicitly provided.

So they are global symbols defined in libraries.  That's all what you've said 
actually means to me.

I think it's clear that there is nothing different about these symbols than any 
others from the perspective of how linking works and how that relates to the 
language specification.  The *only* actual difference is that the language 
specification says that you can override these standard library symbols and 
that you may not override other standard library symbols--a distinction that 
does not actually exist materially in linking, nor matter to a programmer 
following the language specification and not using the compiler in ways outside 
the language specification (such as controlling symbol visibility).

All of your explanations for how they are different have to do with saving 
people from themselves when they asked the compiler to do something non-default 
with all symbols but actually meant to treat these particular symbols 
differently.
AFAICT the rationale goes something like: linking is subtle and hard to 
understand; many people use -fvisibility=hidden and do not fully understand 
what it means; a likely error is failing to understand the need to define a 
function with explicit-default visibility when it should be visible across 
shared library boundaries; conforming C++ code may define its own `operator 
new`/`operator delete` functions to have those used by the standard C++ library 
code (which might be in a shared library) in preference to its own definitions, 
but may not define other standard functions and have the same expectation; 
ergo, people with a poor understanding of linking who have followed some advice 
they didn't fully understand to use `-fvisibility=hidden` and also might define 
their own `operator new`/`operator delete` functions are best served by the 
compiler doing what they meant rather than what they said.

I'm a person who understands linking thoroughly, so I'm not the target audience 
for the concerns you raise.  That's fine.  Since I do understand the issues, I 
have no trouble understanding when I need a special compiler option to get the 
link-time behavior that's correct for my needs.


Repository:
  rC Clang

https://reviews.llvm.org/D53787



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53970: [CMake][Fuchsia] Don't restrict Linux runtimes to UNIX

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D53970



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54026: [CMake][Fuchsia] Set -fuse-ld=lld explicitly for Linux runtimes

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

So the default is not per-target?  It should be.


Repository:
  rC Clang

https://reviews.llvm.org/D54026



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: lib/Driver/ToolChains/Fuchsia.cpp:128
 if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bstatic");
+  CmdArgs.push_back("-static");
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);

MaskRay wrote:
> If Fuchsia doesn't use gold, it is fine. gold diverges from ld.bfd (lld) in 
> that `-static` switches the whole link to its special static mode. (as 
> usually while you link libstdc++/libc++ statically, you can still link other 
> libraries normally)
> 
> In ld.bfd/lld, `-Bstatic` is synonym with `-static`.
I think this part of the change was unintentional and should be undone.
Using `--pop-state` obviates the need for `-Bdynamic` to undo `-Bstatic`, but 
`-Bstatic` is still the right switch here.


Repository:
  rC Clang

https://reviews.llvm.org/D53854



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54064: [Driver] Always match resource dir in Fuchsia driver tests

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D54064



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr reopened this revision.
mcgrathr added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/Driver/ToolChains/Fuchsia.cpp:128
 if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bstatic");
+  CmdArgs.push_back("-static");
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);

mcgrathr wrote:
> MaskRay wrote:
> > If Fuchsia doesn't use gold, it is fine. gold diverges from ld.bfd (lld) in 
> > that `-static` switches the whole link to its special static mode. (as 
> > usually while you link libstdc++/libc++ statically, you can still link 
> > other libraries normally)
> > 
> > In ld.bfd/lld, `-Bstatic` is synonym with `-static`.
> I think this part of the change was unintentional and should be undone.
> Using `--pop-state` obviates the need for `-Bdynamic` to undo `-Bstatic`, but 
> `-Bstatic` is still the right switch here.
Actually, it's wrong two ways: the `--pop-state` should come before `-lm`.  
Neither `-static` nor `-Bstatic` should apply to `-lm` (or to `-lc` that comes 
later, which `-static` might).  `-static` vs `-Bstatic` is only a latent bug 
given lld, but the `-lm` issue breaks the Zircon build today.



Repository:
  rC Clang

https://reviews.llvm.org/D53854



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54082: [Driver] Use -Bstatic/dynamic for libc++ on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D54082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54112: [Driver] Delete redundant -Bdynamic for libc++ on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr requested changes to this revision.
mcgrathr added a comment.
This revision now requires changes to proceed.

This breaks the semantics we want.  The `-Bdynamic` is there to apply to `-lm`, 
which is also what `--as-needed` is there for in this case (it appears earlier 
because of the conditional logic, since in the other case it applies to `-lc++` 
as well).  It's correct as is.


Repository:
  rC Clang

https://reviews.llvm.org/D54112



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54169: [clang-tidy] Zircon ->

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


https://reviews.llvm.org/D54169



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54463: [CMake] Support cross-compiling with Fuchsia toolchain build

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D54463



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55405: [CMake] Use hidden visibility for static libc++ in Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

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

https://reviews.llvm.org/D55405



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60235: [CMake] Disable libc++ and libc++abi new/delete definitions when built with ASan

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

IMHO the library code should use `#if !__has_feature(...)` to avoid the 
definitions entirely when built with a sanitizer whose runtime provides them.
But this is a fine way to achieve that while we wait for those libraries to be 
fixed for sanitized builds.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60235



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61040: [Fuchsia] Support multilib for -fsanitize=address and -fno-exceptions

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1507
+Multilib::flags_list &Flags) {
+  if (Enabled)
+Flags.push_back(std::string("+") + Flag);

I'd have reduced the duplication and just used `Enabled ? "+" : "-"`



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:197
+SmallString<128> P(D.ResourceDir);
+llvm::sys::path::append(P, D.getTargetTriple(), "lib", M.gccSuffix());
+return std::vector({P.str()});

These two lines repeating the path construction logic could be CSEd into a 
lambda.



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:201
+
+  if (const auto &PathsCallback = Multilibs.filePathsCallback())
+for (const auto &Path : PathsCallback(SelectedMultilib))

This merits a comment about the order they're being inserted.



Comment at: clang/test/Driver/fuchsia.cpp:60
+// CHECK-NOEXCEPTIONS-X86: 
"-L[[RESOURCE_DIR]]{{/|}}x86_64-fuchsia{{/|}}lib"
+
+// RUN: %clang %s -### --target=aarch64-fuchsia -fno-exceptions \

Might be worth adding a test that noexcept does *not* appear by default (or 
explicit -fexceptions).


Repository:
  rC Clang

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

https://reviews.llvm.org/D61040



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60990: [Driver] Support priority for multilibs

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: clang/include/clang/Driver/Multilib.h:81
 
+  /// Returns the multilib priority.
+  int priority() const { return Priority; }

Say explicitly that the greatest priority is chosen (sometimes 0 is "best").


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

https://reviews.llvm.org/D60990



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61040: [Fuchsia] Support multilib for -fsanitize=address and -fno-exceptions

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:179
+  .flag("+fno-exceptions");
+  Multilib ASan = Multilib("/asan", "", "", 2).flag("+fsanitize=address");
+  Multilibs.push_back(Default);

Add a comment about the priorities.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61040



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52402: [CMake][Fuchsia] Use internal_linkage rather than always_inline for libc++

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm as long as we revert when the underlying bugs are resovled


Repository:
  rC Clang

https://reviews.llvm.org/D52402



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52660: [CMake][Fuchsia] Use libc++ ABIv2 for Fuchsia toolchain

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D52660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

The motivating use case always pairs `noderef` with an `address_space` 
attribute, and it's already invalid to convert between pointer types in 
different address spaces without a cast.
So I don't think we have a strong opinion one way or the other.  In the 
abstract, I'd say `noderef` feels like a "constraining" qualifier a la 
const/volatile so that going from unconstrained to constrained implicitly is OK 
but not vice versa.


Repository:
  rC Clang

https://reviews.llvm.org/D49511



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58010: [CodeGen] Set construction vtable visibility after creating initializer

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm with a comment (and perhaps an assertion).




Comment at: clang/lib/CodeGen/CGVTables.cpp:777
 
+  CGM.setGVProperties(VTable, RD);
+

Give it a comment pointing out the importance of doing this after 
createVTableInitializer rather than before.
If there's an easy way to add an "assert(is defined)" before setGVProperties to 
go with the comment, do that too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58010



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58325: [Driver][Fuchsia] Support -nolibc flag

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

This is only on GCC trunk (i.e. 9), not in 8.2 or even the current gcc-8 
branch.  So clarify the log entry.  We don't know if 8.x,x>2 will add it or 
only gcc 9.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37723: [Driver] Fuchsia targets default to -fasynchronous-unwind-tables

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
Herald added subscribers: kristof.beyls, aemerson.

This regressed for x86-64 in r307856 because it's no longer inherited
from Generic_GCC.  We'd never noticed that it was missing other
targets (i.e. aarch64), but Fuchsia is uniform across all machines.


Repository:
  rL LLVM

https://reviews.llvm.org/D37723

Files:
  lib/Driver/ToolChains/Fuchsia.h
  test/Driver/fuchsia.c


Index: test/Driver/fuchsia.c
===
--- test/Driver/fuchsia.c
+++ test/Driver/fuchsia.c
@@ -1,6 +1,11 @@
 // RUN: %clang %s -### -no-canonical-prefixes --target=x86_64-unknown-fuchsia \
-// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 | FileCheck %s
+// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
+// RUN: %clang %s -### -no-canonical-prefixes --target=aarch64-unknown-fuchsia 
\
+// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
+// CHECK: "-munwind-tables"
 // CHECK: "-fuse-init-array"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
@@ -15,7 +20,8 @@
 // CHECK-NOT: crti.o
 // CHECK-NOT: crtbegin.o
 // CHECK: "-L[[SYSROOT]]{{/|}}lib"
-// CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a"
+// CHECK-X86_64: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a"
+// CHECK-AARCH64: "{{.*[/\\]}}libclang_rt.builtins-aarch64.a"
 // CHECK: "-lc"
 // CHECK-NOT: crtend.o
 // CHECK-NOT: crtn.o
Index: lib/Driver/ToolChains/Fuchsia.h
===
--- lib/Driver/ToolChains/Fuchsia.h
+++ lib/Driver/ToolChains/Fuchsia.h
@@ -49,6 +49,9 @@
   CXXStdlibType GetDefaultCXXStdlibType() const override {
 return ToolChain::CST_Libcxx;
   }
+  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override {
+return true;
+  }
   bool isPICDefault() const override { return false; }
   bool isPIEDefault() const override { return true; }
   bool isPICDefaultForced() const override { return false; }


Index: test/Driver/fuchsia.c
===
--- test/Driver/fuchsia.c
+++ test/Driver/fuchsia.c
@@ -1,6 +1,11 @@
 // RUN: %clang %s -### -no-canonical-prefixes --target=x86_64-unknown-fuchsia \
-// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 | FileCheck %s
+// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
+// RUN: %clang %s -### -no-canonical-prefixes --target=aarch64-unknown-fuchsia \
+// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
+// CHECK: "-munwind-tables"
 // CHECK: "-fuse-init-array"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
@@ -15,7 +20,8 @@
 // CHECK-NOT: crti.o
 // CHECK-NOT: crtbegin.o
 // CHECK: "-L[[SYSROOT]]{{/|}}lib"
-// CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a"
+// CHECK-X86_64: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a"
+// CHECK-AARCH64: "{{.*[/\\]}}libclang_rt.builtins-aarch64.a"
 // CHECK: "-lc"
 // CHECK-NOT: crtend.o
 // CHECK-NOT: crtn.o
Index: lib/Driver/ToolChains/Fuchsia.h
===
--- lib/Driver/ToolChains/Fuchsia.h
+++ lib/Driver/ToolChains/Fuchsia.h
@@ -49,6 +49,9 @@
   CXXStdlibType GetDefaultCXXStdlibType() const override {
 return ToolChain::CST_Libcxx;
   }
+  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override {
+return true;
+  }
   bool isPICDefault() const override { return false; }
   bool isPIEDefault() const override { return true; }
   bool isPICDefaultForced() const override { return false; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37785: [Fuchsia] Set ENABLE_X86_RELAX_RELOCATIONS for Fuchsia builds

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
Herald added a subscriber: mgorny.

This is a "Does your linker support it?" option, and all ours do.


Repository:
  rL LLVM

https://reviews.llvm.org/D37785

Files:
  cmake/caches/Fuchsia-stage2.cmake


Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -18,6 +18,13 @@
   set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
 endif()
 
+# This is a "Does your linker support it?" option that only applies
+# to x86-64 ELF targets.  All Fuchsia target linkers do support it.
+# For x86-64 Linux, it's supported by LLD and by GNU linkers since
+# binutils 2.27, so one can hope that all Linux hosts in use handle it.
+# Ideally this would be settable as a per-target option.
+set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
+
 if(APPLE)
   set(LLDB_CODESIGN_IDENTITY "" CACHE STRING "")
 endif()


Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -18,6 +18,13 @@
   set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
 endif()
 
+# This is a "Does your linker support it?" option that only applies
+# to x86-64 ELF targets.  All Fuchsia target linkers do support it.
+# For x86-64 Linux, it's supported by LLD and by GNU linkers since
+# binutils 2.27, so one can hope that all Linux hosts in use handle it.
+# Ideally this would be settable as a per-target option.
+set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
+
 if(APPLE)
   set(LLDB_CODESIGN_IDENTITY "" CACHE STRING "")
 endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62712: [CMake][Fuchsia] Use libc++ ABI v2 on Darwin as well

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

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

https://reviews.llvm.org/D62712



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64605: [CMake][Fuchsia] Use RelWithDebInfo to build runtimes

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

Lgtm


Repository:
  rC Clang

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

https://reviews.llvm.org/D64605



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64603: [Target] Use IEEE quad format for long double on Fuchsia x86_64

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/lib/Basic/Targets/X86.h:858
+  : FuchsiaTargetInfo(Triple, Opts) {
+LongDoubleFormat = &llvm::APFloat::IEEEquad();
+  }

Can we just do this in FuchsiaTargetInfo generically?
I think we'd like to make our core API types uniform across machines and this 
makes sure that we get the Fuchsia common choice rather than a machine-specific 
choice that might differ in a new machine.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64603



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65715: [Driver] Don't disable -fsanitizer-coverage for safe-stack or shadow-call-stack

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added reviewers: phosek, MaskRay.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

These "sanitizers" are hardened ABIs that are wholly orthogonal
to the SanitizerCoverage instrumentation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65715

Files:
  clang/lib/Driver/SanitizerArgs.cpp


Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -46,7 +46,8 @@
 SanitizerKind::Undefined | SanitizerKind::Integer |
 SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
 SanitizerKind::DataFlow | SanitizerKind::Fuzzer |
-SanitizerKind::FuzzerNoLink | SanitizerKind::FloatDivideByZero;
+SanitizerKind::FuzzerNoLink | SanitizerKind::FloatDivideByZero |
+SanitizerKind::SafeStack | SanitizerKind::ShadowCallStack;
 static const SanitizerMask RecoverableByDefault =
 SanitizerKind::Undefined | SanitizerKind::Integer |
 SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |


Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -46,7 +46,8 @@
 SanitizerKind::Undefined | SanitizerKind::Integer |
 SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
 SanitizerKind::DataFlow | SanitizerKind::Fuzzer |
-SanitizerKind::FuzzerNoLink | SanitizerKind::FloatDivideByZero;
+SanitizerKind::FuzzerNoLink | SanitizerKind::FloatDivideByZero |
+SanitizerKind::SafeStack | SanitizerKind::ShadowCallStack;
 static const SanitizerMask RecoverableByDefault =
 SanitizerKind::Undefined | SanitizerKind::Integer |
 SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64140: [CMake][Fuchsia] Define asan+noexcept multilib

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

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

https://reviews.llvm.org/D64140



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62442: [Driver] Update handling of c++ and runtime directories

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm contingent on verifying intended behavior changes and adding comments.  
The factoring suggestion for the triple,triple searching is preferred but at 
your discretion, though I'd really like to see at least comments.




Comment at: clang/lib/Driver/Driver.cpp:4428
 
-  if (auto P = SearchPaths(TC.getFilePaths()))
+  if (auto P = SearchPaths(TC.getRuntimePaths()))
 return *P;

I see the renaming Library->Runtime throughout (not clear why, but OK by me).  
This also swaps the order of getFilePaths vs getRuntimePaths, which seems 
significant.  There's a woeful lack of comments in this function about its 
essential logic, which is the order in which it consults all the available 
sources of paths.
So changing the ordering a bit without any comments is concerning.

I'd like to see more comments overall, but as this is a preexisting problem in 
this code, it's enough for this change that you affirm this ordering change was 
intentional and add some commentary somewhere about the material change in 
behavior (here or in the log message).



Comment at: clang/lib/Driver/ToolChain.cpp:389
 SmallString<128> P(LibPath);
 llvm::sys::path::append(P, Prefix + Twine("clang_rt.") + Component + 
Suffix);
+return P.str();

Can you explain the change to  use the first entry rather than the first 
existing entry?
If the first one in the list is presumed to exist, then why is there a list 
instead of a single directory stored?



Comment at: clang/lib/Driver/ToolChain.cpp:410
+  SmallString<128> P;
+
+  P.assign(D.ResourceDir);

Comment about the distinction between D.getTargetTriple() and Triple.str() and 
why this order between them.



Comment at: clang/lib/Driver/ToolChain.cpp:424
+
+Optional ToolChain::getCXXStdlibPath() const {
+  SmallString<128> P;

Same here.  Can these use a common subroutine/template for trying something 
based on these two, so the logic about what D.getTargetTriple() and 
Triple.str() mean and both the code and justifying comments for their ordering 
logic is de-duplicated?



Comment at: clang/test/Driver/fuchsia.c:96
 // CHECK-ASAN-X86: "-dynamic-linker" "asan/ld.so.1"
-// CHECK-ASAN-X86: 
"-L[[RESOURCE_DIR]]{{/|}}x86_64-fuchsia{{/|}}lib{{/|}}asan"
-// CHECK-ASAN-X86: "-L[[RESOURCE_DIR]]{{/|}}x86_64-fuchsia{{/|}}lib"

To clarify, these are dropped because their sole intent was always to serve 
-lc++ and that's properly not enabled for this C-only link?


Repository:
  rC Clang

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

https://reviews.llvm.org/D62442



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62469: [Driver] Change layout of per-target runtimes to resemble multiarch

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62469



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62558: [Driver] Search the toolchain dir with -print-file-name

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

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

https://reviews.llvm.org/D62558



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56348: [CMake][Fuchsia] Enable --build-id linker flag by default

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

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

https://reviews.llvm.org/D56348



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56350: [CMake][Fuchsia] Enable experimental new pass manager by default

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

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

https://reviews.llvm.org/D56350



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56349: [CMake][Fuchsia] Enable x86 relaxation by default

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm.  Wouldn't hurt to comment that this depends on a linker new enough to 
support the new reloc types (only reason it's conditional at all) and since we 
default to lld we don't worry about host system linkers that might be too old 
to support the new reloc types.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56349



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56359: [CMake][Fuchsia] Enable build ID, relaxations for first stage

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

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

https://reviews.llvm.org/D56359



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46361: [CMake][Cache] Stop pretending that Fuchsia is UNIX

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

fnu lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D46361



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48563: [CMake] Use explicit targets for building Linux runtimes

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D48563



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48564: [CMake] Support passing FUCHSIA_SDK as the only variable

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D48564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42019: [Driver] Set default sysroot for Fuchsia if none is specified

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

Is this still live? Should it be different after all the multiarch stuff?


Repository:
  rC Clang

https://reviews.llvm.org/D42019



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66712: [Driver] Enable ShadowCallStack, not SafeStack, by default on AArch64 Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

It's time to land this now.  Can one of you commit it for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66712



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66712: [Driver] Enable ShadowCallStack, not SafeStack, by default on AArch64 Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 225273.
mcgrathr added a comment.

rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66712

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -13,7 +13,8 @@
 // CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: "-fsanitize=safe-stack"
+// CHECK-AARCH64: "-fsanitize=shadow-call-stack"
+// CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK: "-fno-common"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic" "-z" 
"separate-loadable-segments"
@@ -102,7 +103,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-ASAN-AARCH64
 // CHECK-ASAN-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-ASAN-AARCH64: "-fsanitize=address"
+// CHECK-ASAN-AARCH64: "-fsanitize=address,shadow-call-stack"
 // CHECK-ASAN-AARCH64: "-fsanitize-address-globals-dead-stripping"
 // CHECK-ASAN-AARCH64: "-dynamic-linker" "asan/ld.so.1"
 // CHECK-ASAN-AARCH64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.asan.so"
@@ -134,7 +135,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-FUZZER-AARCH64
 // CHECK-FUZZER-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link,safe-stack"
+// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link,shadow-call-stack"
 // CHECK-FUZZER-AARCH64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.fuzzer.a"
 
 // RUN: %clang %s -### --target=x86_64-fuchsia \
@@ -153,7 +154,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-SCUDO-AARCH64
 // CHECK-SCUDO-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-SCUDO-AARCH64: "-fsanitize=safe-stack,scudo"
+// CHECK-SCUDO-AARCH64: "-fsanitize=shadow-call-stack,scudo"
 // CHECK-SCUDO-AARCH64: "-pie"
 // CHECK-SCUDO-AARCH64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.scudo.so"
 
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -343,5 +343,10 @@
 }
 
 SanitizerMask Fuchsia::getDefaultSanitizers() const {
-  return SanitizerKind::SafeStack;
+  SanitizerMask Res;
+  if (getTriple().getArch() == llvm::Triple::aarch64)
+Res |= SanitizerKind::ShadowCallStack;
+  else
+Res |= SanitizerKind::SafeStack;
+  return Res;
 }


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -13,7 +13,8 @@
 // CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: "-fsanitize=safe-stack"
+// CHECK-AARCH64: "-fsanitize=shadow-call-stack"
+// CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK: "-fno-common"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic" "-z" "separate-loadable-segments"
@@ -102,7 +103,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-ASAN-AARCH64
 // CHECK-ASAN-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-ASAN-AARCH64: "-fsanitize=address"
+// CHECK-ASAN-AARCH64: "-fsanitize=address,shadow-call-stack"
 // CHECK-ASAN-AARCH64: "-fsanitize-address-globals-dead-stripping"
 // CHECK-ASAN-AARCH64: "-dynamic-linker" "asan/ld.so.1"
 // CHECK-ASAN-AARCH64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.asan.so"
@@ -134,7 +135,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-FUZZER-AARCH64
 // CHECK-FUZZER-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link,safe-stack"
+// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link,shadow-call-stack"
 // CHECK-FUZZER-AARCH64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.fuzzer.a"
 
 // RUN: %clang %s -### --target=x86_64-fuchsia \
@@ -153,7 +154,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-SCUDO-AARCH64
 // CHECK-SCUDO-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-SCUDO-AARCH64: "-fsanitize=safe-stack,scudo"
+// CHECK-SCUDO-AARCH64: "-fsanitize=shadow-call-stack,scudo"
 // CHECK-SCUDO-AARCH64: "-pie"
 // CHECK-SCUDO-AARCH64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.scudo.so"
 
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
==

[PATCH] D79835: [Fuchsia] Rely on linker switch rather than dead code ref for profile runtime

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 267336.
mcgrathr added a comment.

comment update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79835

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Fuchsia.h
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.profile.a
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.profile.a
  clang/test/Driver/fuchsia.c
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/linkage.ll


Index: llvm/test/Instrumentation/InstrProfiling/linkage.ll
===
--- llvm/test/Instrumentation/InstrProfiling/linkage.ll
+++ llvm/test/Instrumentation/InstrProfiling/linkage.ll
@@ -2,9 +2,11 @@
 
 ; RUN: opt < %s -mtriple=x86_64-apple-macosx10.10.0 -instrprof -S | FileCheck 
%s --check-prefixes=POSIX,MACHO
 ; RUN: opt < %s -mtriple=x86_64-unknown-linux -instrprof -S | FileCheck %s 
--check-prefixes=POSIX,LINUX
+; RUN: opt < %s -mtriple=x86_64-unknown-fuchsia -instrprof -S | FileCheck %s 
--check-prefixes=POSIX,LINUX
 ; RUN: opt < %s  -mtriple=x86_64-pc-win32-coff -instrprof -S | FileCheck %s 
--check-prefixes=COFF
 ; RUN: opt < %s -mtriple=x86_64-apple-macosx10.10.0 -passes=instrprof -S | 
FileCheck %s --check-prefixes=POSIX,MACHO
 ; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes=instrprof -S | FileCheck 
%s --check-prefixes=POSIX,LINUX
+; RUN: opt < %s -mtriple=x86_64-unknown-fuchsia -passes=instrprof -S | 
FileCheck %s --check-prefixes=POSIX,LINUX
 ; RUN: opt < %s  -mtriple=x86_64-pc-win32-coff -passes=instrprof -S | 
FileCheck %s --check-prefixes=COFF
 
 ; MACHO: @__llvm_profile_runtime = external global i32
Index: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
===
--- llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -1034,9 +1034,9 @@
 }
 
 bool InstrProfiling::emitRuntimeHook() {
-  // We expect the linker to be invoked with -u flag for linux,
-  // for which case there is no need to emit the user function.
-  if (TT.isOSLinux())
+  // We expect the linker to be invoked with -u flag for Linux or
+  // Fuchsia, in which case there is no need to emit the user function.
+  if (TT.isOSLinux() || TT.isOSFuchsia())
 return false;
 
   // If the module's provided its own runtime, we don't need to do anything.
Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -242,3 +242,21 @@
 // RUN: -gsplit-dwarf -c %s 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-SPLIT-DWARF
 // CHECK-SPLIT-DWARF: "-split-dwarf-output" "fuchsia.dwo"
+
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fprofile-instr-generate -fcoverage-mapping \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-PROFRT-AARCH64
+// CHECK-PROFRT-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PROFRT-AARCH64: "-u__llvm_profile_runtime"
+// CHECK-PROFRT-AARCH64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.profile.a"
+
+// RUN: %clang %s -### --target=x86_64-fuchsia \
+// RUN: -fprofile-instr-generate -fcoverage-mapping \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-PROFRT-X86_64
+// CHECK-PROFRT-X86_64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PROFRT-X86_64: "-u__llvm_profile_runtime"
+// CHECK-PROFRT-X86_64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.profile.a"
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -69,6 +69,9 @@
   SanitizerMask getSupportedSanitizers() const override;
   SanitizerMask getDefaultSanitizers() const override;
 
+  void addProfileRTLibs(const llvm::opt::ArgList &Args,
+llvm::opt::ArgStringList &CmdArgs) const override;
+
   RuntimeLibType
   GetRuntimeLibType(const llvm::opt::ArgList &Args) const override;
   CXXStdlibType
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -15,6 +15,7 @@
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Suppor

[PATCH] D84482: [Fuchsia] Use -z dead-reloc-in-nonalloc=*=0 for Fuchsia targets

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added reviewers: phosek, leonardchan.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84482

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp


Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -53,6 +53,13 @@
   CmdArgs.push_back("-z");
   CmdArgs.push_back("now");
 
+  // 0 is never a valid code address for Fuchsia (user or kernel), so using the
+  // traditional 0 value for references to discarded code is always fine on
+  // Fuchsia.  The new defaults of -1 and such in lld confuse some older tools
+  // for no benefit to Fuchsia.
+  CmdArgs.push_back("-z");
+  CmdArgs.push_back("dead-reloc-in-nonalloc=*=0");
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   if (llvm::sys::path::filename(Exec).equals_lower("ld.lld") ||
   llvm::sys::path::stem(Exec).equals_lower("ld.lld")) {


Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -53,6 +53,13 @@
   CmdArgs.push_back("-z");
   CmdArgs.push_back("now");
 
+  // 0 is never a valid code address for Fuchsia (user or kernel), so using the
+  // traditional 0 value for references to discarded code is always fine on
+  // Fuchsia.  The new defaults of -1 and such in lld confuse some older tools
+  // for no benefit to Fuchsia.
+  CmdArgs.push_back("-z");
+  CmdArgs.push_back("dead-reloc-in-nonalloc=*=0");
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   if (llvm::sys::path::filename(Exec).equals_lower("ld.lld") ||
   llvm::sys::path::stem(Exec).equals_lower("ld.lld")) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78687: [Fuchsia] Build compiler-rt builtins for 32-bit x86

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78687



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85362: [CMake] Print the autodetected host linker version

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

That's a great start.  IMHO we should give the driver a way to print out its 
embedded value too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85362

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/include/clang/Basic/TargetCXXABI.h:39
 
+  static const auto &getABIMap() {
+static llvm::StringMap ABIMap = {

The option UI should use lowercase values by default, or else just do 
case-insensitive matching.




Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:274
+
+  if (DriverArgs.getLastArgValue(options::OPT_fcxx_abi_EQ) == "Fuchsia")
+CC1Args.push_back("-fexperimental-relative-c++-abi-vtables");

This should match "fuchsia", either only lowercase or case-insensitive.  IMHO 
it seems wise to handle this in some way such that individual tests like this 
are not free-form string comparisons that could have typos, but test the 
TargetCXXABI enum value after decoding from string.

This should be a separate change.  We'll need to do staged roll-out, first of a 
compiler where `-fc++abi=compat` (or itanium whatever it's called) is available 
(complete with multilibs et al), and then later of a compiler where 
`-fc++abi=fuchsia` (and the default for *-fuchsia targets) has changed meaning.




Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:275
+  if (DriverArgs.getLastArgValue(options::OPT_fcxx_abi_EQ) == "Fuchsia")
+CC1Args.push_back("-fexperimental-relative-c++-abi-vtables");
 }

It's surprising to me that this is the way to do this.  Isn't there code in the 
actual front end that tests the TargetCXXABI value?  That seems like the place 
where it makes sense to have Fuchsia imply specific settings, rather than in 
the driver.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85924: [clang] Add __RELATIVE_CXX_ABI_VTABLES predefine macro

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:1127
 
+  if (LangOpts.RelativeCXXABIVTables)
+Builder.defineMacro("__RELATIVE_CXX_ABI_VTABLES");

This should also test `LangOpts.CplusPlus` so it's never defined in C 
compilation.

I don't know what precedents there are to follow for how to choose the exact 
name here.
Having something like `__CXX_ABI_` be a prefix for all such things seems wise.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85924

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85924: [clang] Add __RELATIVE_CXX_ABI_VTABLES predefine macro

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:1127
 
+  if (LangOpts.RelativeCXXABIVTables)
+Builder.defineMacro("__RELATIVE_CXX_ABI_VTABLES");

ldionne wrote:
> mcgrathr wrote:
> > This should also test `LangOpts.CplusPlus` so it's never defined in C 
> > compilation.
> > 
> > I don't know what precedents there are to follow for how to choose the 
> > exact name here.
> > Having something like `__CXX_ABI_` be a prefix for all such things seems 
> > wise.
> > 
> Should we be using some `__has_feature(...)` instead?
That does seem like a very good fit here, and really easy to use with just one 
line in Features.def AFAICT.
Maybe `__has_feature(cxx_abi_relative_vtable)` and in future add more 
`cxx_abi_foo` ones?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85924

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85924: [clang][feature] Add cxx_abi_relative_vtable feature

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

LGTM but I'm not sure who should sign off on new `__has_feature` symbols.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85924

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79665: [Clang] Pass --pack-dyn-relocs=relr to lld for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added a reviewer: phosek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The compact format is fully supported on Fuchsia and is the
preferred default.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79665

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" 
"separate-loadable-segments"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" 
"separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -56,6 +56,7 @@
 CmdArgs.push_back("rodynamic");
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
+CmdArgs.push_back("--pack-dyn-relocs=relr");
   }
 
   if (!D.SysRoot.empty())


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -56,6 +56,7 @@
 CmdArgs.push_back("rodynamic");
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
+CmdArgs.push_back("--pack-dyn-relocs=relr");
   }
 
   if (!D.SysRoot.empty())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79667: [Clang] Pass -z max-page-size to linker for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added a reviewer: phosek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
mcgrathr added a parent revision: D79665: [Clang] Pass --pack-dyn-relocs=relr 
to lld for Fuchsia.

Currently all Fuchsia ABIs use a 4k page size, departing from
the recommended page sizes in the respective psABI documents.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79667

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" 
"separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -46,6 +46,9 @@
   // handled somewhere else.
   Args.ClaimAllArgs(options::OPT_w);
 
+  CmdArgs.push_back("-z");
+  CmdArgs.push_back("max-page-size=4096");
+
   CmdArgs.push_back("-z");
   CmdArgs.push_back("now");
 


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -46,6 +46,9 @@
   // handled somewhere else.
   Args.ClaimAllArgs(options::OPT_w);
 
+  CmdArgs.push_back("-z");
+  CmdArgs.push_back("max-page-size=4096");
+
   CmdArgs.push_back("-z");
   CmdArgs.push_back("now");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79835: [Fuchsia] Rely on linker switch rather than dead code ref for profile runtime

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added a reviewer: phosek.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

Follow the model used on Linux, where the clang driver passes the
linker a `-u` switch to force the profile runtime to be linked in,
rather than having every TU emit a dead function with a reference.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79835

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Fuchsia.h
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.profile.a
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.profile.a
  clang/test/Driver/fuchsia.c
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/linkage.ll


Index: llvm/test/Instrumentation/InstrProfiling/linkage.ll
===
--- llvm/test/Instrumentation/InstrProfiling/linkage.ll
+++ llvm/test/Instrumentation/InstrProfiling/linkage.ll
@@ -2,9 +2,11 @@
 
 ; RUN: opt < %s -mtriple=x86_64-apple-macosx10.10.0 -instrprof -S | FileCheck 
%s --check-prefixes=POSIX,MACHO
 ; RUN: opt < %s -mtriple=x86_64-unknown-linux -instrprof -S | FileCheck %s 
--check-prefixes=POSIX,LINUX
+; RUN: opt < %s -mtriple=x86_64-unknown-fuchsia -instrprof -S | FileCheck %s 
--check-prefixes=POSIX,LINUX
 ; RUN: opt < %s  -mtriple=x86_64-pc-win32-coff -instrprof -S | FileCheck %s 
--check-prefixes=COFF
 ; RUN: opt < %s -mtriple=x86_64-apple-macosx10.10.0 -passes=instrprof -S | 
FileCheck %s --check-prefixes=POSIX,MACHO
 ; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes=instrprof -S | FileCheck 
%s --check-prefixes=POSIX,LINUX
+; RUN: opt < %s -mtriple=x86_64-unknown-fuchsia -passes=instrprof -S | 
FileCheck %s --check-prefixes=POSIX,LINUX
 ; RUN: opt < %s  -mtriple=x86_64-pc-win32-coff -passes=instrprof -S | 
FileCheck %s --check-prefixes=COFF
 
 ; MACHO: @__llvm_profile_runtime = external global i32
Index: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
===
--- llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -1036,7 +1036,7 @@
 bool InstrProfiling::emitRuntimeHook() {
   // We expect the linker to be invoked with -u flag for linux,
   // for which case there is no need to emit the user function.
-  if (TT.isOSLinux())
+  if (TT.isOSLinux() || TT.isOSFuchsia())
 return false;
 
   // If the module's provided its own runtime, we don't need to do anything.
Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -242,3 +242,21 @@
 // RUN: -gsplit-dwarf -c %s 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-SPLIT-DWARF
 // CHECK-SPLIT-DWARF: "-split-dwarf-output" "fuchsia.dwo"
+
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fprofile-instr-generate -fcoverage-mapping \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-PROFRT-AARCH64
+// CHECK-PROFRT-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PROFRT-AARCH64: "-u__llvm_profile_runtime"
+// CHECK-PROFRT-AARCH64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.profile.a"
+
+// RUN: %clang %s -### --target=x86_64-fuchsia \
+// RUN: -fprofile-instr-generate -fcoverage-mapping \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-PROFRT-X86_64
+// CHECK-PROFRT-X86_64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PROFRT-X86_64: "-u__llvm_profile_runtime"
+// CHECK-PROFRT-X86_64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.profile.a"
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -69,6 +69,9 @@
   SanitizerMask getSupportedSanitizers() const override;
   SanitizerMask getDefaultSanitizers() const override;
 
+  void addProfileRTLibs(const llvm::opt::ArgList &Args,
+llvm::opt::ArgStringList &CmdArgs) const override;
+
   RuntimeLibType
   GetRuntimeLibType(const llvm::opt::ArgList &Args) const override;
   CXXStdlibType
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -15,6 +15,7 @@
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/ProfileData/Instr

[PATCH] D40329: [CMake][Fuchsia] Disable terminfo database in Fuchsia toolchain

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rL LLVM

https://reviews.llvm.org/D40329



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122613: [Driver] Make -moutline-atomics default for aarch64-fuchsia targets

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added reviewers: phosek, abrachet.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
mcgrathr requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

This makes Fuchsia consistent with Linux on AArch64.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122613

Files:
  clang/lib/Driver/ToolChains/Fuchsia.h
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -34,6 +34,7 @@
 // CHECK-AARCH64: "-fsanitize=shadow-call-stack"
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
+// CHECK-AARCH64: "-target-feature" "+outline-atomics"
 // CHECK-NOT: "-fcommon"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "-z" "rel" 
"--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -75,24 +75,27 @@
 
   RuntimeLibType
   GetRuntimeLibType(const llvm::opt::ArgList &Args) const override;
-  CXXStdlibType
-  GetCXXStdlibType(const llvm::opt::ArgList &Args) const override;
+  CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList &Args) const 
override;
+
+  bool IsAArch64OutlineAtomicsDefault(
+  const llvm::opt::ArgList &Args) const override {
+return true;
+  }
 
-  void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args,
- Action::OffloadKind DeviceOffloadKind) const 
override;
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const override;
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
-  void
-  AddClangCXXStdlibIncludeArgs(const llvm::opt::ArgList &DriverArgs,
-   llvm::opt::ArgStringList &CC1Args) const 
override;
+  void AddClangCXXStdlibIncludeArgs(
+  const llvm::opt::ArgList &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) const override;
 
-  const char *getDefaultLinker() const override {
-return "ld.lld";
-  }
+  const char *getDefaultLinker() const override { return "ld.lld"; }
 
 protected:
   Tool *buildLinker() const override;


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -34,6 +34,7 @@
 // CHECK-AARCH64: "-fsanitize=shadow-call-stack"
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
+// CHECK-AARCH64: "-target-feature" "+outline-atomics"
 // CHECK-NOT: "-fcommon"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "-z" "rel" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -75,24 +75,27 @@
 
   RuntimeLibType
   GetRuntimeLibType(const llvm::opt::ArgList &Args) const override;
-  CXXStdlibType
-  GetCXXStdlibType(const llvm::opt::ArgList &Args) const override;
+  CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList &Args) const override;
+
+  bool IsAArch64OutlineAtomicsDefault(
+  const llvm::opt::ArgList &Args) const override {
+return true;
+  }
 
-  void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args,
- Action::OffloadKind DeviceOffloadKind) const override;
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const override;
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
-  void
-  AddClangCXXStdlibIncludeArgs(const llvm::opt::ArgList &DriverArgs,
-   llvm::opt::ArgStringList &CC1Args) const override;
+  void AddClangCXXStdlibIncludeArgs(
+  const llvm::opt::ArgList &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) const override;
   void AddCXXStdlibLibArgs(const llvm::op

[PATCH] D122613: [Driver] Make -moutline-atomics default for aarch64-fuchsia targets

[Roland McGrath via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1a963d3278c2: [Driver] Make -moutline-atomics default for 
aarch64-fuchsia targets (authored by mcgrathr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122613

Files:
  clang/lib/Driver/ToolChains/Fuchsia.h
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -34,6 +34,7 @@
 // CHECK-AARCH64: "-fsanitize=shadow-call-stack"
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
+// CHECK-AARCH64: "-target-feature" "+outline-atomics"
 // CHECK-NOT: "-fcommon"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "-z" "rel" 
"--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -75,24 +75,27 @@
 
   RuntimeLibType
   GetRuntimeLibType(const llvm::opt::ArgList &Args) const override;
-  CXXStdlibType
-  GetCXXStdlibType(const llvm::opt::ArgList &Args) const override;
+  CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList &Args) const 
override;
+
+  bool IsAArch64OutlineAtomicsDefault(
+  const llvm::opt::ArgList &Args) const override {
+return true;
+  }
 
-  void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args,
- Action::OffloadKind DeviceOffloadKind) const 
override;
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const override;
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
-  void
-  AddClangCXXStdlibIncludeArgs(const llvm::opt::ArgList &DriverArgs,
-   llvm::opt::ArgStringList &CC1Args) const 
override;
+  void AddClangCXXStdlibIncludeArgs(
+  const llvm::opt::ArgList &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) const override;
 
-  const char *getDefaultLinker() const override {
-return "ld.lld";
-  }
+  const char *getDefaultLinker() const override { return "ld.lld"; }
 
 protected:
   Tool *buildLinker() const override;


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -34,6 +34,7 @@
 // CHECK-AARCH64: "-fsanitize=shadow-call-stack"
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
+// CHECK-AARCH64: "-target-feature" "+outline-atomics"
 // CHECK-NOT: "-fcommon"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "-z" "rel" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -75,24 +75,27 @@
 
   RuntimeLibType
   GetRuntimeLibType(const llvm::opt::ArgList &Args) const override;
-  CXXStdlibType
-  GetCXXStdlibType(const llvm::opt::ArgList &Args) const override;
+  CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList &Args) const override;
+
+  bool IsAArch64OutlineAtomicsDefault(
+  const llvm::opt::ArgList &Args) const override {
+return true;
+  }
 
-  void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args,
- Action::OffloadKind DeviceOffloadKind) const override;
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const override;
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
-  void
-  AddClangCXXStdlibIncludeArgs(const llvm::opt::ArgList &DriverArgs,
-   llvm::opt::ArgStringList &CC1Args) const override;
+  void AddClangCXXStdlibIncludeArgs(
+  const llvm::opt::ArgList &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) const override;
   void AddCXXStdlibLibArgs(const llvm::opt:

[PATCH] D119201: [clang][Fuchsia] Ensure static sanitizer libs are only linked in after the -nostdlib check

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:130
 
-  bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
   bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);

I think all these runtime additions need to be conditional.  There must be no 
kinds of implicit link inputs at all when using -nostdlib.
I'm not sure what matters or doesn't wrt their relative order vs 
AddLinkerInputs.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119201

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119201: [clang][Fuchsia] Ensure static sanitizer libs are only linked in after the -nostdlib check

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:157
 if (NeedsSanitizerDeps)
   linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 

This function is a no-op because it tests for fuchsia targets.  So it probably 
makes more sense not to call it and to remove its fuchsia special-case.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119201

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129512: [Driver] Don't use frame pointer on Fuchsia when optimizations are enabled

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

lgtm but be sure that Fuchsia target users get clear release-notes warning 
about the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129512

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66712: [Driver] Enable ShadowCallStack, not SafeStack, by default on AArch64 Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
Herald added subscribers: cfe-commits, cryptoad, kristof.beyls, javed.absar.
Herald added a project: clang.

On AArch64, Fuchsia fully supports both SafeStack and ShadowCallStack ABIs.
The latter is now preferred and will be the default.  It's possible to
enable both simultaneously, but ShadowCallStack is believed to have most
of the practical benefit of SafeStack with less cost.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66712

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -13,7 +13,8 @@
 // CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: "-fsanitize=safe-stack"
+// CHECK-AARCH64: "-fsanitize=shadow-call-stack"
+// CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK: "-fno-common"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic" "-z" "separate-code"
@@ -102,7 +103,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-ASAN-AARCH64
 // CHECK-ASAN-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-ASAN-AARCH64: "-fsanitize=address"
+// CHECK-ASAN-AARCH64: "-fsanitize=address,shadow-call-stack"
 // CHECK-ASAN-AARCH64: "-fsanitize-address-globals-dead-stripping"
 // CHECK-ASAN-AARCH64: "-dynamic-linker" "asan/ld.so.1"
 // CHECK-ASAN-AARCH64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.asan.so"
@@ -134,7 +135,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-FUZZER-AARCH64
 // CHECK-FUZZER-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link,safe-stack"
+// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link,shadow-call-stack"
 // CHECK-FUZZER-AARCH64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.fuzzer.a"
 
 // RUN: %clang %s -### --target=x86_64-fuchsia \
@@ -153,7 +154,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-SCUDO-AARCH64
 // CHECK-SCUDO-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-SCUDO-AARCH64: "-fsanitize=safe-stack,scudo"
+// CHECK-SCUDO-AARCH64: "-fsanitize=shadow-call-stack,scudo"
 // CHECK-SCUDO-AARCH64: "-pie"
 // CHECK-SCUDO-AARCH64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.scudo.so"
 
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -343,5 +343,10 @@
 }
 
 SanitizerMask Fuchsia::getDefaultSanitizers() const {
-  return SanitizerKind::SafeStack;
+  SanitizerMask Res;
+  if (getTriple().getArch() == llvm::Triple::aarch64)
+Res |= SanitizerKind::ShadowCallStack;
+  else
+Res |= SanitizerKind::SafeStack;
+  return Res;
 }


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -13,7 +13,8 @@
 // CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: "-fsanitize=safe-stack"
+// CHECK-AARCH64: "-fsanitize=shadow-call-stack"
+// CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK: "-fno-common"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic" "-z" "separate-code"
@@ -102,7 +103,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-ASAN-AARCH64
 // CHECK-ASAN-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-ASAN-AARCH64: "-fsanitize=address"
+// CHECK-ASAN-AARCH64: "-fsanitize=address,shadow-call-stack"
 // CHECK-ASAN-AARCH64: "-fsanitize-address-globals-dead-stripping"
 // CHECK-ASAN-AARCH64: "-dynamic-linker" "asan/ld.so.1"
 // CHECK-ASAN-AARCH64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.asan.so"
@@ -134,7 +135,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-FUZZER-AARCH64
 // CHECK-FUZZER-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link,safe-stack"
+// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link,shadow-call-stack"
 // CHECK-FUZZER-AARCH64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.fuzzer.a"
 
 // RUN: %clang %s -### --target=x86_64-fuchsia \
@@ -153,7 +154,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-SCUDO-AARCH64
 // CHECK-SCUDO-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-SCUDO-AARCH64: "-fsanitize=safe-stack,scudo"
+// CHECK-SCUDO-A

[PATCH] D66712: [Driver] Enable ShadowCallStack, not SafeStack, by default on AArch64 Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added reviewers: phosek, leonardchan, jakehehrlich.
mcgrathr added a comment.

This should land only after we've done our first stages of downstream roll-out 
and burn-in testing.  But setting it up now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66712



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35922: [Driver] Enable AddressSanitizer for Fuchsia targets

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: cmake/caches/Fuchsia-stage2.cmake:50
   set(RUNTIMES_${target}-fuchsia_LIBCXX_ABI_VERSION 2 CACHE STRING "")
+  set(RUNTIMES_${target}-fuchsia_SANITIZER_USE_COMPILER_RT ON CACHE BOOL "")
 endforeach()

phosek wrote:
> I think we also need:
> ```
> set(RUNTIMES_${target}-fuchsia_SANITIZER_CXX_ABI "libcxxabi" ON CACHE STRING 
> "")
> ```
> 
> 
That seems to default correctly when building it all together.
Should we add it anyway?


Repository:
  rL LLVM

https://reviews.llvm.org/D35922



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36202: [Driver] Disable static C++ library support on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

LGTM.  The nit below is orthogonal to this change.




Comment at: lib/Driver/ToolChains/Fuchsia.cpp:112
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
-if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bdynamic");
-  }
   CmdArgs.push_back("-lm");
 }

Shouldn't -lm be conditionalized on ShouldLinkCXXStdlib too?
It's only there to satisfy references from libc++, right?

OTOH, -lm is a no-op on Fuchsia so perhaps we should just drop it.  On the 
third hand, we might separate libm from libc in the future so perhaps it's 
better to leave it.


Repository:
  rL LLVM

https://reviews.llvm.org/D36202



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35922: [Driver] Enable AddressSanitizer for Fuchsia targets

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 109428.
mcgrathr added a comment.

Added tests.


https://reviews.llvm.org/D35922

Files:
  cmake/caches/Fuchsia-stage2.cmake
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChains/Fuchsia.cpp
  test/Driver/fuchsia.c

Index: test/Driver/fuchsia.c
===
--- test/Driver/fuchsia.c
+++ test/Driver/fuchsia.c
@@ -44,3 +44,27 @@
 // RUN: -fsanitize=safe-stack 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-SAFESTACK
 // CHECK-SAFESTACK: "-fsanitize=safe-stack"
+
+// RUN: %clang %s -### --target=x86_64-unknown-fuchsia \
+// RUN: -fsanitize=address 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-ASAN-X86
+// CHECK-ASAN-X86: "-fsanitize=address"
+// CHECK-ASAN-X86: "-fsanitize-address-globals-dead-stripping"
+// CHECK-ASAN-X86: /libclang_rt.asan-x86_64.so
+// CHECK-ASAN-X86: /libclang_rt.asan-preinit-x86_64.a
+
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fsanitize=address 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-ASAN-AARCH64
+// CHECK-ASAN-AARCH64: "-fsanitize=address"
+// CHECK-ASAN-AARCH64: "-fsanitize-address-globals-dead-stripping"
+// CHECK-ASAN-AARCH64: /libclang_rt.asan-aarch64.so
+// CHECK-ASAN-AARCH64: /libclang_rt.asan-preinit-aarch64.a
+
+// RUN: %clang %s -### --target=x86_64-unknown-fuchsia \
+// RUN: -fsanitize=address -fPIC -shared 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-ASAN-SHARED
+// CHECK-ASAN-SHARED: "-fsanitize=address"
+// CHECK-ASAN-SHARED: "-fsanitize-address-globals-dead-stripping"
+// CHECK-ASAN-SHARED: /libclang_rt.asan-x86_64.so
+// CHECK-ASAN-SHARED-NOT: /libclang_rt.asan-preinit-x86_64.a
Index: lib/Driver/ToolChains/Fuchsia.cpp
===
--- lib/Driver/ToolChains/Fuchsia.cpp
+++ lib/Driver/ToolChains/Fuchsia.cpp
@@ -14,6 +14,7 @@
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
+#include "clang/Driver/SanitizerArgs.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/Path.h"
 
@@ -68,22 +69,21 @@
   else
 CmdArgs.push_back("--build-id");
 
-  if (!Args.hasArg(options::OPT_static))
-CmdArgs.push_back("--eh-frame-hdr");
+  CmdArgs.push_back("--eh-frame-hdr");
 
   if (Args.hasArg(options::OPT_static))
 CmdArgs.push_back("-Bstatic");
   else if (Args.hasArg(options::OPT_shared))
 CmdArgs.push_back("-shared");
 
-  if (!Args.hasArg(options::OPT_static)) {
-if (Args.hasArg(options::OPT_rdynamic))
-  CmdArgs.push_back("-export-dynamic");
-
-if (!Args.hasArg(options::OPT_shared)) {
-  CmdArgs.push_back("-dynamic-linker");
-  CmdArgs.push_back(Args.MakeArgString(D.DyldPrefix + "ld.so.1"));
-}
+  if (!Args.hasArg(options::OPT_shared)) {
+std::string Dyld = D.DyldPrefix;
+if (ToolChain.getSanitizerArgs().needsAsanRt() &&
+ToolChain.getSanitizerArgs().needsSharedAsanRt())
+  Dyld += "asan/";
+Dyld += "ld.so.1";
+CmdArgs.push_back("-dynamic-linker");
+CmdArgs.push_back(Args.MakeArgString(Dyld));
   }
 
   CmdArgs.push_back("-o");
@@ -100,6 +100,8 @@
 
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
+  addSanitizerRuntimes(ToolChain, Args, CmdArgs);
+
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
@@ -277,5 +279,6 @@
 SanitizerMask Fuchsia::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::SafeStack;
+  Res |= SanitizerKind::Address;
   return Res;
 }
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -171,7 +171,7 @@
   return ((Sanitizers.Mask & NeedsUbsanRt & ~TrapSanitizers.Mask) ||
   CoverageFeatures) &&
  !Sanitizers.has(Address) && !Sanitizers.has(Memory) &&
- !Sanitizers.has(Thread) && !Sanitizers.has(DataFlow) && 
+ !Sanitizers.has(Thread) && !Sanitizers.has(DataFlow) &&
  !Sanitizers.has(Leak) && !CfiCrossDso;
 }
 
@@ -557,8 +557,9 @@
 
   if (AllAddedKinds & Address) {
 AsanSharedRuntime =
-Args.hasArg(options::OPT_shared_libasan) || TC.getTriple().isAndroid();
-NeedPIE |= TC.getTriple().isAndroid();
+Args.hasArg(options::OPT_shared_libasan) ||
+TC.getTriple().isAndroid() || TC.getTriple().isOSFuchsia();
+NeedPIE |= TC.getTriple().isAndroid() || TC.getTriple().isOSFuchsia();
 if (Arg *A =
 Args.getLastArg(options::OPT_fsanitize_address_field_padding)) {
 StringRef S = A->getValue();
@@ -592,7 +593,7 @@
 // globals in ASan is disabled by default on ELF targets.
 // See https://sourceware.org/bugzilla/show_bug.cgi?id=19002
 AsanGlobalsDeadStripping =
-!TC.getTriple().isOSBinFormatELF() ||
+!TC.getTriple().isOSBinFormat

[PATCH] D36254: [Driver][Fuchsia] Pass --hash-style=gnu to the linker

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.

The .gnu_hash format is superior, and all versions of the Fuchsia
dynamic linker support it.


Repository:
  rL LLVM

https://reviews.llvm.org/D36254

Files:
  lib/Driver/ToolChains/Fuchsia.cpp


Index: lib/Driver/ToolChains/Fuchsia.cpp
===
--- lib/Driver/ToolChains/Fuchsia.cpp
+++ lib/Driver/ToolChains/Fuchsia.cpp
@@ -65,8 +65,10 @@
 
   if (Args.hasArg(options::OPT_r))
 CmdArgs.push_back("-r");
-  else
+  else {
 CmdArgs.push_back("--build-id");
+CmdArgs.push_back("--hash-style=gnu");
+  }
 
   if (!Args.hasArg(options::OPT_static))
 CmdArgs.push_back("--eh-frame-hdr");


Index: lib/Driver/ToolChains/Fuchsia.cpp
===
--- lib/Driver/ToolChains/Fuchsia.cpp
+++ lib/Driver/ToolChains/Fuchsia.cpp
@@ -65,8 +65,10 @@
 
   if (Args.hasArg(options::OPT_r))
 CmdArgs.push_back("-r");
-  else
+  else {
 CmdArgs.push_back("--build-id");
+CmdArgs.push_back("--hash-style=gnu");
+  }
 
   if (!Args.hasArg(options::OPT_static))
 CmdArgs.push_back("--eh-frame-hdr");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36254: [Driver][Fuchsia] Pass --hash-style=gnu to the linker

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 109603.
mcgrathr added a comment.

Added test.


https://reviews.llvm.org/D36254

Files:
  lib/Driver/ToolChains/Fuchsia.cpp
  test/Driver/fuchsia.c


Index: test/Driver/fuchsia.c
===
--- test/Driver/fuchsia.c
+++ test/Driver/fuchsia.c
@@ -9,6 +9,7 @@
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
+// CHECK: "--hash-style=gnu"
 // CHECK: "-dynamic-linker" "ld.so.1"
 // CHECK: Scrt1.o
 // CHECK-NOT: crti.o
Index: lib/Driver/ToolChains/Fuchsia.cpp
===
--- lib/Driver/ToolChains/Fuchsia.cpp
+++ lib/Driver/ToolChains/Fuchsia.cpp
@@ -65,8 +65,10 @@
 
   if (Args.hasArg(options::OPT_r))
 CmdArgs.push_back("-r");
-  else
+  else {
 CmdArgs.push_back("--build-id");
+CmdArgs.push_back("--hash-style=gnu");
+  }
 
   if (!Args.hasArg(options::OPT_static))
 CmdArgs.push_back("--eh-frame-hdr");


Index: test/Driver/fuchsia.c
===
--- test/Driver/fuchsia.c
+++ test/Driver/fuchsia.c
@@ -9,6 +9,7 @@
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
+// CHECK: "--hash-style=gnu"
 // CHECK: "-dynamic-linker" "ld.so.1"
 // CHECK: Scrt1.o
 // CHECK-NOT: crti.o
Index: lib/Driver/ToolChains/Fuchsia.cpp
===
--- lib/Driver/ToolChains/Fuchsia.cpp
+++ lib/Driver/ToolChains/Fuchsia.cpp
@@ -65,8 +65,10 @@
 
   if (Args.hasArg(options::OPT_r))
 CmdArgs.push_back("-r");
-  else
+  else {
 CmdArgs.push_back("--build-id");
+CmdArgs.push_back("--hash-style=gnu");
+  }
 
   if (!Args.hasArg(options::OPT_static))
 CmdArgs.push_back("--eh-frame-hdr");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35922: [Driver] Enable AddressSanitizer for Fuchsia targets

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 109605.
mcgrathr added a comment.

Check -dynamic-linker in test.


https://reviews.llvm.org/D35922

Files:
  cmake/caches/Fuchsia-stage2.cmake
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChains/Fuchsia.cpp
  test/Driver/fuchsia.c

Index: test/Driver/fuchsia.c
===
--- test/Driver/fuchsia.c
+++ test/Driver/fuchsia.c
@@ -44,3 +44,29 @@
 // RUN: -fsanitize=safe-stack 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-SAFESTACK
 // CHECK-SAFESTACK: "-fsanitize=safe-stack"
+
+// RUN: %clang %s -### --target=x86_64-unknown-fuchsia \
+// RUN: -fsanitize=address 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-ASAN-X86
+// CHECK-ASAN-X86: "-fsanitize=address"
+// CHECK-ASAN-X86: "-fsanitize-address-globals-dead-stripping"
+// CHECK-ASAN-X86: "-dynamic-linker" "asan/ld.so.1"
+// CHECK-ASAN-X86: /libclang_rt.asan-x86_64.so
+// CHECK-ASAN-X86: /libclang_rt.asan-preinit-x86_64.a
+
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fsanitize=address 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-ASAN-AARCH64
+// CHECK-ASAN-AARCH64: "-fsanitize=address"
+// CHECK-ASAN-AARCH64: "-fsanitize-address-globals-dead-stripping"
+// CHECK-ASAN-AARCH64: "-dynamic-linker" "asan/ld.so.1"
+// CHECK-ASAN-AARCH64: /libclang_rt.asan-aarch64.so
+// CHECK-ASAN-AARCH64: /libclang_rt.asan-preinit-aarch64.a
+
+// RUN: %clang %s -### --target=x86_64-unknown-fuchsia \
+// RUN: -fsanitize=address -fPIC -shared 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-ASAN-SHARED
+// CHECK-ASAN-SHARED: "-fsanitize=address"
+// CHECK-ASAN-SHARED: "-fsanitize-address-globals-dead-stripping"
+// CHECK-ASAN-SHARED: /libclang_rt.asan-x86_64.so
+// CHECK-ASAN-SHARED-NOT: /libclang_rt.asan-preinit-x86_64.a
Index: lib/Driver/ToolChains/Fuchsia.cpp
===
--- lib/Driver/ToolChains/Fuchsia.cpp
+++ lib/Driver/ToolChains/Fuchsia.cpp
@@ -14,6 +14,7 @@
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
+#include "clang/Driver/SanitizerArgs.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/Path.h"
 
@@ -68,22 +69,21 @@
   else
 CmdArgs.push_back("--build-id");
 
-  if (!Args.hasArg(options::OPT_static))
-CmdArgs.push_back("--eh-frame-hdr");
+  CmdArgs.push_back("--eh-frame-hdr");
 
   if (Args.hasArg(options::OPT_static))
 CmdArgs.push_back("-Bstatic");
   else if (Args.hasArg(options::OPT_shared))
 CmdArgs.push_back("-shared");
 
-  if (!Args.hasArg(options::OPT_static)) {
-if (Args.hasArg(options::OPT_rdynamic))
-  CmdArgs.push_back("-export-dynamic");
-
-if (!Args.hasArg(options::OPT_shared)) {
-  CmdArgs.push_back("-dynamic-linker");
-  CmdArgs.push_back(Args.MakeArgString(D.DyldPrefix + "ld.so.1"));
-}
+  if (!Args.hasArg(options::OPT_shared)) {
+std::string Dyld = D.DyldPrefix;
+if (ToolChain.getSanitizerArgs().needsAsanRt() &&
+ToolChain.getSanitizerArgs().needsSharedAsanRt())
+  Dyld += "asan/";
+Dyld += "ld.so.1";
+CmdArgs.push_back("-dynamic-linker");
+CmdArgs.push_back(Args.MakeArgString(Dyld));
   }
 
   CmdArgs.push_back("-o");
@@ -100,6 +100,8 @@
 
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
+  addSanitizerRuntimes(ToolChain, Args, CmdArgs);
+
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
@@ -277,5 +279,6 @@
 SanitizerMask Fuchsia::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::SafeStack;
+  Res |= SanitizerKind::Address;
   return Res;
 }
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -171,7 +171,7 @@
   return ((Sanitizers.Mask & NeedsUbsanRt & ~TrapSanitizers.Mask) ||
   CoverageFeatures) &&
  !Sanitizers.has(Address) && !Sanitizers.has(Memory) &&
- !Sanitizers.has(Thread) && !Sanitizers.has(DataFlow) && 
+ !Sanitizers.has(Thread) && !Sanitizers.has(DataFlow) &&
  !Sanitizers.has(Leak) && !CfiCrossDso;
 }
 
@@ -557,8 +557,9 @@
 
   if (AllAddedKinds & Address) {
 AsanSharedRuntime =
-Args.hasArg(options::OPT_shared_libasan) || TC.getTriple().isAndroid();
-NeedPIE |= TC.getTriple().isAndroid();
+Args.hasArg(options::OPT_shared_libasan) ||
+TC.getTriple().isAndroid() || TC.getTriple().isOSFuchsia();
+NeedPIE |= TC.getTriple().isAndroid() || TC.getTriple().isOSFuchsia();
 if (Arg *A =
 Args.getLastArg(options::OPT_fsanitize_address_field_padding)) {
 StringRef S = A->getValue();
@@ -592,7 +593,7 @@
 // globals in ASan is disabled by default on ELF targets.
 // See https://sourceware.org/bugzilla/show_bug.cg

[PATCH] D36349: [CMake] Build sanitized C++ runtimes for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: cmake/caches/Fuchsia-stage2.cmake:56
 
+foreach(target x86_64;aarch64)
+  set(RUNTIMES_${target}-fuchsia-asan_CMAKE_BUILD_WITH_INSTALL_RPATH ON CACHE 
BOOL "")

Can you do this without duplicating all the identical settings from the block 
above?
It looks like the asan cases are just all of the above plus two more.


Repository:
  rL LLVM

https://reviews.llvm.org/D36349



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36779: [Driver] SafeStack does not need a runtime library on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added a project: Sanitizers.

Repository:
  rL LLVM

https://reviews.llvm.org/D36779

Files:
  include/clang/Driver/SanitizerArgs.h
  lib/Driver/SanitizerArgs.cpp
  test/Driver/fuchsia.c


Index: test/Driver/fuchsia.c
===
--- test/Driver/fuchsia.c
+++ test/Driver/fuchsia.c
@@ -45,6 +45,8 @@
 // RUN: -fsanitize=safe-stack 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-SAFESTACK
 // CHECK-SAFESTACK: "-fsanitize=safe-stack"
+// CHECK-SAFESTACK-NOT: "{{.*[/\\]}}libclang_rt.safestack-x86_64.a"
+// CHECK-SAFESTACK-NOT: "__safestack_init"
 
 // RUN: %clang %s -### --target=x86_64-unknown-fuchsia \
 // RUN: -fsanitize=address 2>&1 \
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -605,6 +605,11 @@
 AsanUseAfterScope = false;
   }
 
+  if (AllAddedKinds & SafeStack) {
+// SafeStack runtime is built into the system on Fuchsia.
+SafeStackRuntime = !TC.getTriple().isOSFuchsia();
+  }
+
   // Parse -link-cxx-sanitizer flag.
   LinkCXXRuntimes =
   Args.hasArg(options::OPT_fsanitize_link_cxx_runtime) || D.CCCIsCXX();
Index: include/clang/Driver/SanitizerArgs.h
===
--- include/clang/Driver/SanitizerArgs.h
+++ include/clang/Driver/SanitizerArgs.h
@@ -38,6 +38,7 @@
   bool AsanGlobalsDeadStripping = false;
   bool LinkCXXRuntimes = false;
   bool NeedPIE = false;
+  bool SafeStackRuntime = false;
   bool Stats = false;
   bool TsanMemoryAccess = true;
   bool TsanFuncEntryExit = true;
@@ -58,9 +59,7 @@
   }
   bool needsUbsanRt() const;
   bool needsDfsanRt() const { return Sanitizers.has(SanitizerKind::DataFlow); }
-  bool needsSafeStackRt() const {
-return Sanitizers.has(SanitizerKind::SafeStack);
-  }
+  bool needsSafeStackRt() const { return SafeStackRuntime; }
   bool needsCfiRt() const;
   bool needsCfiDiagRt() const;
   bool needsStatsRt() const { return Stats; }


Index: test/Driver/fuchsia.c
===
--- test/Driver/fuchsia.c
+++ test/Driver/fuchsia.c
@@ -45,6 +45,8 @@
 // RUN: -fsanitize=safe-stack 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-SAFESTACK
 // CHECK-SAFESTACK: "-fsanitize=safe-stack"
+// CHECK-SAFESTACK-NOT: "{{.*[/\\]}}libclang_rt.safestack-x86_64.a"
+// CHECK-SAFESTACK-NOT: "__safestack_init"
 
 // RUN: %clang %s -### --target=x86_64-unknown-fuchsia \
 // RUN: -fsanitize=address 2>&1 \
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -605,6 +605,11 @@
 AsanUseAfterScope = false;
   }
 
+  if (AllAddedKinds & SafeStack) {
+// SafeStack runtime is built into the system on Fuchsia.
+SafeStackRuntime = !TC.getTriple().isOSFuchsia();
+  }
+
   // Parse -link-cxx-sanitizer flag.
   LinkCXXRuntimes =
   Args.hasArg(options::OPT_fsanitize_link_cxx_runtime) || D.CCCIsCXX();
Index: include/clang/Driver/SanitizerArgs.h
===
--- include/clang/Driver/SanitizerArgs.h
+++ include/clang/Driver/SanitizerArgs.h
@@ -38,6 +38,7 @@
   bool AsanGlobalsDeadStripping = false;
   bool LinkCXXRuntimes = false;
   bool NeedPIE = false;
+  bool SafeStackRuntime = false;
   bool Stats = false;
   bool TsanMemoryAccess = true;
   bool TsanFuncEntryExit = true;
@@ -58,9 +59,7 @@
   }
   bool needsUbsanRt() const;
   bool needsDfsanRt() const { return Sanitizers.has(SanitizerKind::DataFlow); }
-  bool needsSafeStackRt() const {
-return Sanitizers.has(SanitizerKind::SafeStack);
-  }
+  bool needsSafeStackRt() const { return SafeStackRuntime; }
   bool needsCfiRt() const;
   bool needsCfiDiagRt() const;
   bool needsStatsRt() const { return Stats; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86822: [clang] Enable -fsanitize=thread on Fuchsia.

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.

I'm not 100% sure we don't need more fiddles in the driver, but we can iterate 
from here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86822

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90275: [clang][IR] Add support for leaf attribute

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

The GCC documentation specifically gives the example of the standard C function 
`qsort` as one that does not qualify as `__attribute__((leaf))` because it uses 
a callback function (that presumably might be from the caller's own TU).  AIUI 
the claim the attribute makes is that no normal control flow (i.e. excluding 
only signal handlers) would call any function in the caller's TU by any means, 
nor `longjmp` to any state captured by a `setjmp` call in the caller's TU, and 
thus only normal return or normal C++ exception handling return/unwind would 
reach the caller's TU again after the jump (call) to this entry point.  The 
caller is thus presumed not to share potentially-called functions with the 
callee by any means, whether direct symbol references or function pointers 
passed in the call or function pointers previously stored somewhere (e.g. a C++ 
vtable).  The compiler can make whatever assumptions about potential 
dataflow/side-effect and the like are implied by this exclusion of TU-local 
code paths from the otherwise unknown call graph of the external call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90275

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91226: [clang] Add missing header guard in

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added reviewers: phosek, leonardchan.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
mcgrathr requested review of this revision.

This header has long lacked a standard multiple inclusion guard
like other headers have, for no apparent reason.  The GCC header
of the same name likewise lacks one up through release 10.1, but
trunk GCC (release 11, and perhaps future 10.x) has fixed it
(see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96238).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91226

Files:
  clang/lib/Headers/cpuid.h
  clang/test/Headers/cpuid.c


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 %s -ffreestanding -triple i386 -emit-llvm -o - | FileCheck 
%s --check-prefix=CHECK-32
 
 #include 
+#include  // Make sure multiple inclusion protection works.
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A 
cpuid\0A xchgq %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -7,6 +7,9 @@
  *===---===
  */
 
+#ifndef __CPUID_H
+#define __CPUID_H
+
 #if !(__x86_64__ || __i386__)
 #error this header is for x86 only
 #endif
@@ -312,3 +315,5 @@
 __cpuid_count(__leaf, __subleaf, *__eax, *__ebx, *__ecx, *__edx);
 return 1;
 }
+
+#endif /* __CPUID_H */


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 %s -ffreestanding -triple i386 -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-32
 
 #include 
+#include  // Make sure multiple inclusion protection works.
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A cpuid\0A xchgq %rbx,${1:q}", "={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -7,6 +7,9 @@
  *===---===
  */
 
+#ifndef __CPUID_H
+#define __CPUID_H
+
 #if !(__x86_64__ || __i386__)
 #error this header is for x86 only
 #endif
@@ -312,3 +315,5 @@
 __cpuid_count(__leaf, __subleaf, *__eax, *__ebx, *__ecx, *__edx);
 return 1;
 }
+
+#endif /* __CPUID_H */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91226: [clang] Add missing header guard in

[Roland McGrath via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcf36142d342a: [clang] Add missing header guard in 
 (authored by mcgrathr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91226

Files:
  clang/lib/Headers/cpuid.h
  clang/test/Headers/cpuid.c


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 %s -ffreestanding -triple i386 -emit-llvm -o - | FileCheck 
%s --check-prefix=CHECK-32
 
 #include 
+#include  // Make sure multiple inclusion protection works.
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A 
cpuid\0A xchgq %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -7,6 +7,9 @@
  *===---===
  */
 
+#ifndef __CPUID_H
+#define __CPUID_H
+
 #if !(__x86_64__ || __i386__)
 #error this header is for x86 only
 #endif
@@ -312,3 +315,5 @@
 __cpuid_count(__leaf, __subleaf, *__eax, *__ebx, *__ecx, *__edx);
 return 1;
 }
+
+#endif /* __CPUID_H */


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 %s -ffreestanding -triple i386 -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-32
 
 #include 
+#include  // Make sure multiple inclusion protection works.
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A cpuid\0A xchgq %rbx,${1:q}", "={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -7,6 +7,9 @@
  *===---===
  */
 
+#ifndef __CPUID_H
+#define __CPUID_H
+
 #if !(__x86_64__ || __i386__)
 #error this header is for x86 only
 #endif
@@ -312,3 +315,5 @@
 __cpuid_count(__leaf, __subleaf, *__eax, *__ebx, *__ecx, *__edx);
 return 1;
 }
+
+#endif /* __CPUID_H */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91387: [Driver] Support UBSan multilib

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:214-215
 
+
set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LLVM_BUILD_COMPILER_RT 
OFF CACHE BOOL "")
+set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LLVM_USE_SANITIZER 
"Undefined" CACHE STRING "")
+
set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS
 OFF CACHE BOOL "")

Don't you need plain +ubsan instances of these like for +asan above?




Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:216-217
+set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LLVM_USE_SANITIZER 
"Undefined" CACHE STRING "")
+
set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS
 OFF CACHE BOOL "")
+
set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS
 OFF CACHE BOOL "")
+
set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LIBCXXABI_ENABLE_EXCEPTIONS
 OFF CACHE BOOL "")

Standalone ubsan doesn't replace the allocator, so these shouldn't be 
overridden for it like they are for asan.




Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:205
   .flag("+fno-exceptions"));
+  // UBSan has lower priority than ASan so we pick the latter when both are 
set.
+  Multilibs.push_back(Multilib("ubsan", {}, {}, 2)

This being the case, should we be adding `-fsanitize=undefined` to the libc++ 
(et al) build for asan?
I guess that's an orthogonal change, really.




Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:217
   // Use the asan+noexcept variant with ASan and -fno-exceptions.
-  Multilibs.push_back(Multilib("asan+noexcept", {}, {}, 3)
+  Multilibs.push_back(Multilib("asan+noexcept", {}, {}, 5)
   .flag("+fsanitize=address")

This is getting to be a lot of individual ordered integer constants to maintain.
Can we just define a local enum to represent the ordering and use symbolic 
names in each call?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91387

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92444: [CMake][Fuchsia] Install llvm-elfabi

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added reviewers: leonardchan, haowei.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.
mcgrathr requested review of this revision.

The canonical Fuchsia toolchain configuration installs llvm-elfabi.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92444

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -245,6 +245,7 @@
   llvm-dlltool
   llvm-dwarfdump
   llvm-dwp
+  llvm-elfabi
   llvm-gsymutil
   llvm-lib
   llvm-mt


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -245,6 +245,7 @@
   llvm-dlltool
   llvm-dwarfdump
   llvm-dwp
+  llvm-elfabi
   llvm-gsymutil
   llvm-lib
   llvm-mt
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92444: [CMake][Fuchsia] Install llvm-elfabi

[Roland McGrath via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG70764c02e474: [CMake][Fuchsia] Install llvm-elfabi (authored 
by mcgrathr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92444

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -245,6 +245,7 @@
   llvm-dlltool
   llvm-dwarfdump
   llvm-dwp
+  llvm-elfabi
   llvm-gsymutil
   llvm-lib
   llvm-mt


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -245,6 +245,7 @@
   llvm-dlltool
   llvm-dwarfdump
   llvm-dwp
+  llvm-elfabi
   llvm-gsymutil
   llvm-lib
   llvm-mt
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90275: [clang][IR] Add support for leaf attribute

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

There is an RFC going out with this prototype as reference.  When there is 
consensus on the RFC, this will get in shape for landing with complete tests 
and all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90275

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103544: [compiler-rt][Fuchsia] Disable interceptors while enabling new/delete replacements

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: compiler-rt/lib/hwasan/CMakeLists.txt:45
+if (NOT FUCHSIA)
+  append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS 
HWASAN_WITH_INTERCEPTORS=1 HWASAN_DEFINITIONS)
+else()

It might be better to force the value of COMPILER_RT_HWASAN_WITH_INTERCEPTORS 
to OFF instead.

But I'll leave the cmake details to Petr.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103544

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103543: [compiler-rt][Fuchsia] Support HWASan on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

I think this may need to be about the last thing to actually land.  It 
shouldn't land before building the runtime reliably works without error, and I 
think we should land any refactoring of the runtime we needed in preparation 
for Fuchsia-specific changes before landing anything Fuchsia-specific to make 
it compile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103543

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103555: [Fuchsia] Use libc++abi on Windows in Fuchsia toolchain

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103555

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103564: [NFC][compiler-rt][hwasan] Wrap vfork interceptor with HWASAN_WITH_INTERCEPTORS

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

I think probably the better refactoring here would be to split this file in 
two: one containing just the allocation functions; and a second with everything 
else.
On Fuchsia, we'll use the allocation functions but none of the rest of it.  So 
the second whole file could just be under `#if !SANITIZER_FUCHSIA`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103564

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93668: [clang] Override the Fuchsia platform ABI using the triple environment

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

I am also a C++ programmer working on C++ ABI support.  I'm glad to be working 
on a platform that made many intentional design decisions to enable keeping the 
C++ ABI out of the platform ABI.  It makes it vastly easier to do interesting 
work on C++ ABIs such as what Leo is doing.  It also makes it significantly 
easier to do interesting work on platform ABIs, which I also do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93668

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103564: [NFC][compiler-rt][hwasan] Wrap fork interceptor with HWASAN_WITH_INTERCEPTORS

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

In D103564#2797657 , @leonardchan 
wrote:

> I don't think we can move the `__sanitizer_*` allocation functions since 
> they're used for aliases for their libc equivalents which require definitions 
> in the same TU:

What I suggested is moving *everything* related to the allocation functions 
into a separate file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103564

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits