Move getenv for AS_SECURE_LOG_FILE to clang

[benlangmuir]

Summary

Avoid calling getenv in the MC layer and let the clang driver do it so that it is reflected in the command-line as an -mllvm option.

rdar://101558354

[benlangmuir]

llvm/lib/MC/MCContext.cpp

61–62

Interestingly, opt<char*> doesn't have a parser; this code only worked with the default value before.

[steven_wu]

If we have to fix it, I slightly prefer just give it a proper option in CodeGenOption and a cc1 and cc1as flag.

llvm/lib/MC/MCContext.cpp

61–62

ha, maybe we should just deprecate the directive since this hasn't been working for more than 6 years!

[benlangmuir]

llvm/lib/MC/MCContext.cpp

61–62

It works if you use the environment variable because that feeds directly into the default value for the cl::opt. It's only if you actually tried to pass -mllvm -as-secure-log-file-name ... it would fail to parse the option.

[benlangmuir]

• Updated to use cc1 option in clang
• Moved the llvm-mc option to a central location so it can be used when initialized MCTargetOptions
• Added tests to cover cc1 and cc1as since each one passes in the string to MC in a slightly different way (CodeGenOpts vs. AssemblerInvocation)

[MaskRay]

clang/test/Driver/AS_SECURE_LOG_FILE.s

1

--target= for new tests

remove unused -o %t.o

llvm/lib/MC/MCContext.cpp

77

When is TargetOptions null? When it is TargetOptions, Does a value of "" matter?

[steven_wu]

LGTM except the comments from @MaskRay

[benlangmuir]

• --target=, remove -o per review
• Use "log_file" as path to try to avoid Windows path issue in driver test

llvm/lib/MC/MCContext.cpp

77

When is TargetOptions null?

null is the default value for the parameter, and happens in various places that construct MCOptions. None of them require a secure log file as far as I can tell.

Does a value of "" matter?

Like the other file path in MCTargetOptions (SplitDwarfFile) empty indicates no file specified.

[benlangmuir]

llvm/lib/MC/MCContext.cpp

77

@MaskRay was this just a question, or did you want me to change something here? Wanted to check before I merge this.

[MaskRay]

llvm/lib/MC/MCContext.cpp

77

Just a question. This looks good.

The environment variable use is a bit odd. I do not know whether it is a legacy feature...

clang/include/clang/Basic/CodeGenOptions.h

public:
   ///
   /// If threshold option is not specified, it is disabled by default.
   Optional<uint64_t> DiagnosticsHotnessThreshold = 0;
 
   /// The maximum percentage profiling weights can deviate from the expected
   /// values in order to be included in misexpect diagnostics.
   Optional<uint32_t> DiagnosticsMisExpectTolerance = 0;
 
+  /// The name of a file to use with \c .secure_log_unique directives.
+  std::string AsSecureLogFile;
+
 public:
   // Define accessors/mutators for code generation options of enumeration type.
 #define CODEGENOPT(Name, Bits, Default)
 #define ENUM_CODEGENOPT(Name, Type, Bits, Default) \
   Type get##Name() const { return static_cast<Type>(Name); } \
   void set##Name(Type Value) { Name = static_cast<unsigned>(Value); }
 #include "clang/Basic/CodeGenOptions.def"
 

clang/include/clang/Driver/Options.td

def mno_type_check : Flag<["-"], "mno-type-check">,
   MarshallingInfoFlag<CodeGenOpts<"NoTypeCheck">>;
 def fno_math_builtin : Flag<["-"], "fno-math-builtin">,
   HelpText<"Disable implicit builtin knowledge of math functions">,
   MarshallingInfoFlag<LangOpts<"NoMathBuiltin">>;
 def fno_use_ctor_homing: Flag<["-"], "fno-use-ctor-homing">,
     HelpText<"Don't use constructor homing for debug info">;
 def fuse_ctor_homing: Flag<["-"], "fuse-ctor-homing">,
     HelpText<"Use constructor homing if we are using limited debug info already">;
+def as_secure_log_file : Separate<["-"], "as-secure-log-file">,
+  HelpText<"Emit .secure_log_unique directives to this filename.">,
+  MarshallingInfoString<CodeGenOpts<"AsSecureLogFile">>;
 
 } // let Flags = [CC1Option, CC1AsOption, NoDriverOption]
 
 let Flags = [CC1Option, NoDriverOption] in {
 
 def disable_llvm_verifier : Flag<["-"], "disable-llvm-verifier">,
   HelpText<"Don't run the LLVM IR verifier pass">,
   MarshallingInfoNegativeFlag<CodeGenOpts<"VerifyModule">>;

clang/lib/CodeGen/BackendUtil.cpp

for (const auto &Entry : HSOpts.UserEntries)
     if (!Entry.IsFramework &&
         (Entry.Group == frontend::IncludeDirGroup::Quoted ||
          Entry.Group == frontend::IncludeDirGroup::Angled ||
          Entry.Group == frontend::IncludeDirGroup::System))
       Options.MCOptions.IASSearchPaths.push_back(
           Entry.IgnoreSysRoot ? Entry.Path : HSOpts.Sysroot + Entry.Path);
   Options.MCOptions.Argv0 = CodeGenOpts.Argv0;
   Options.MCOptions.CommandLineArgs = CodeGenOpts.CommandLineArgs;
+  Options.MCOptions.AsSecureLogFile = CodeGenOpts.AsSecureLogFile;
   Options.MisExpect = CodeGenOpts.MisExpect;
 
   return true;
 }
 
 static Optional<GCOVOptions> getGCOVOptions(const CodeGenOptions &CodeGenOpts,
                                             const LangOptions &LangOpts) {
   if (!CodeGenOpts.EmitGcovArcs && !CodeGenOpts.EmitGcovNotes)

clang/lib/Driver/ToolChains/Clang.cpp

if (MipsTargetFeature != nullptr) {
     CmdArgs.push_back("-target-feature");
     CmdArgs.push_back(MipsTargetFeature);
   }
 
   // forward -fembed-bitcode to assmebler
   if (C.getDriver().embedBitcodeEnabled() ||
       C.getDriver().embedBitcodeMarkerOnly())
     Args.AddLastArg(CmdArgs, options::OPT_fembed_bitcode_EQ);
+
+  if (const char *AsSecureLogFile = getenv("AS_SECURE_LOG_FILE")) {
+    CmdArgs.push_back("-as-secure-log-file");
+    CmdArgs.push_back(Args.MakeArgString(AsSecureLogFile));
+  }
 }
 
 static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
                                        bool OFastEnabled, const ArgList &Args,
                                        ArgStringList &CmdArgs,
                                        const JobAction &JA) {
   // Handle various floating point optimization flags, mapping them to the
   // appropriate LLVM code generation flags. This is complicated by several

clang/test/CodeGen/as-secure-log-file.c

+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-obj %s -o %t.o -as-secure-log-file %t.log
+// RUN: FileCheck %s -input-file %t.log
+// CHECK: "foobar"
+
+void test(void) {
+  __asm__(".secure_log_unique \"foobar\"");
+}

clang/test/Driver/AS_SECURE_LOG_FILE.s

+// RUN: env AS_SECURE_LOG_FILE=%t %clang -target x86_64-apple-darwin -c %s -o %t.o -### 2>&1 | FileCheck %s -DLOG_FILE=%t
+// CHECK: "-cc1as"
+// CHECK-SAME: "-as-secure-log-file" "[[LOG_FILE]]"

clang/test/Misc/cc1as-as-secure-log-file.s

+// RUN: %clang -cc1as -triple x86_64-apple-darwin %s -o %t.o -as-secure-log-file %t.log
+// RUN: FileCheck %s -input-file %t.log
+// CHECK: "foobar"
+
+.secure_log_unique "foobar"

clang/tools/driver/cc1as_main.cpp

struct AssemblerInvocation {
 
   /// Darwin target variant triple, the variant of the deployment target
   /// for which the code is being compiled.
   llvm::Optional<llvm::Triple> DarwinTargetVariantTriple;
 
   /// The version of the darwin target variant SDK which was used during the
   /// compilation
   llvm::VersionTuple DarwinTargetVariantSDKVersion;
+
+  /// The name of a file to use with \c .secure_log_unique directives.
+  std::string AsSecureLogFile;
   /// @}
 
 public:
   AssemblerInvocation() {
     Triple = "";
     NoInitialTextSection = 0;
     InputFile = "-";
     OutputPath = "-";
bool AssemblerInvocation::CreateFromArgs(AssemblerInvocation &Opts,
   if (auto *A = Args.getLastArg(OPT_femit_dwarf_unwind_EQ)) {
     Opts.EmitDwarfUnwind =
         llvm::StringSwitch<EmitDwarfUnwindType>(A->getValue())
             .Case("always", EmitDwarfUnwindType::Always)
             .Case("no-compact-unwind", EmitDwarfUnwindType::NoCompactUnwind)
             .Case("default", EmitDwarfUnwindType::Default);
   }
 
+  Opts.AsSecureLogFile = Args.getLastArgValue(OPT_as_secure_log_file);
+
   return Success;
 }
 
 static std::unique_ptr<raw_fd_ostream>
 getOutputStream(StringRef Path, DiagnosticsEngine &Diags, bool Binary) {
   // Make sure that the Out file gets unlinked from the disk if we get a
   // SIGINT.
   if (Path != "-")
static bool ExecuteAssemblerImpl(AssemblerInvocation &Opts,
   // it later.
   SrcMgr.setIncludeDirs(Opts.IncludePaths);
 
   std::unique_ptr<MCRegisterInfo> MRI(TheTarget->createMCRegInfo(Opts.Triple));
   assert(MRI && "Unable to create target register info!");
 
   MCTargetOptions MCOptions;
   MCOptions.EmitDwarfUnwind = Opts.EmitDwarfUnwind;
+  MCOptions.AsSecureLogFile = Opts.AsSecureLogFile;
 
   std::unique_ptr<MCAsmInfo> MAI(
       TheTarget->createMCAsmInfo(*MRI, Opts.Triple, MCOptions));
   assert(MAI && "Unable to create target asm info!");
 
   // Ensure MCAsmInfo initialization occurs before any use, otherwise sections
   // may be created with a combination of default and explicit settings.
   MAI->setCompressDebugSections(Opts.CompressDebugSections);

llvm/include/llvm/MC/MCContext.h

private:
   unsigned GetInstance(unsigned LocalLabelVal);
 
   /// LLVM_BB_ADDR_MAP version to emit.
   uint8_t BBAddrMapVersion = 1;
 
   /// The file name of the log file from the environment variable
   /// AS_SECURE_LOG_FILE.  Which must be set before the .secure_log_unique
   /// directive is used or it is an error.
-  char *SecureLogFile;
+  std::string SecureLogFile;
   /// The stream that gets written to for the .secure_log_unique directive.
   std::unique_ptr<raw_fd_ostream> SecureLog;
   /// Boolean toggled when .secure_log_unique / .secure_log_reset is seen to
   /// catch errors if .secure_log_unique appears twice without
   /// .secure_log_reset appearing between them.
   bool SecureLogUsed = false;
 
   /// The compilation directory to use for DW_AT_comp_dir.
public:
   void setDwarfFormat(dwarf::DwarfFormat f) { DwarfFormat = f; }
   dwarf::DwarfFormat getDwarfFormat() const { return DwarfFormat; }
 
   void setDwarfVersion(uint16_t v) { DwarfVersion = v; }
   uint16_t getDwarfVersion() const { return DwarfVersion; }
 
   /// @}
 
-  char *getSecureLogFile() { return SecureLogFile; }
+  StringRef getSecureLogFile() { return SecureLogFile; }
   raw_fd_ostream *getSecureLog() { return SecureLog.get(); }
 
   void setSecureLog(std::unique_ptr<raw_fd_ostream> Value) {
     SecureLog = std::move(Value);
   }
 
   bool getSecureLogUsed() { return SecureLogUsed; }
   void setSecureLogUsed(bool Value) { SecureLogUsed = Value; }

llvm/include/llvm/MC/MCTargetOptions.h

enum DwarfDirectory {
     // Default is based on the target
     DefaultDwarfDirectory
   };
   DwarfDirectory MCUseDwarfDirectory;
 
   std::string ABIName;
   std::string AssemblyLanguage;
   std::string SplitDwarfFile;
+  std::string AsSecureLogFile;
 
   const char *Argv0 = nullptr;
   ArrayRef<std::string> CommandLineArgs;
 
   /// Additional paths to search for `.include` directives when using the
   /// integrated assembler.
   std::vector<std::string> IASSearchPaths;
 

llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h

 bool getNoWarn();
 
 bool getNoDeprecatedWarn();
 
 bool getNoTypeCheck();
 
 std::string getABIName();
 
+std::string getAsSecureLogFile();
+
 /// Create this object with static storage to register mc-related command
 /// line options.
 struct RegisterMCTargetOptionsFlags {
   RegisterMCTargetOptionsFlags();
 };
 
 MCTargetOptions InitMCTargetOptionsFromFlags();
 
 } // namespace mc
 
 } // namespace llvm
 
 #endif

llvm/lib/MC/MCContext.cpp

 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <cstdlib>
 #include <tuple>
 #include <utility>
 
 using namespace llvm;
 
-static cl::opt<char*>
-AsSecureLogFileName("as-secure-log-file-name",
-        cl::desc("As secure log file name (initialized from "
-                 "AS_SECURE_LOG_FILE env variable)"),
-        cl::init(getenv("AS_SECURE_LOG_FILE")), cl::Hidden);
-
 static void defaultDiagHandler(const SMDiagnostic &SMD, bool, const SourceMgr &,
                                std::vector<const MDNode *> &) {
   SMD.print(nullptr, errs());
 }
 
 MCContext::MCContext(const Triple &TheTriple, const MCAsmInfo *mai,
                      const MCRegisterInfo *mri, const MCSubtargetInfo *msti,
                      const SourceMgr *mgr, MCTargetOptions const *TargetOpts,
                      bool DoAutoReset, StringRef Swift5ReflSegmentName)
     : Swift5ReflectionSegmentName(Swift5ReflSegmentName), TT(TheTriple),
       SrcMgr(mgr), InlineSrcMgr(nullptr), DiagHandler(defaultDiagHandler),
       MAI(mai), MRI(mri), MSTI(msti), Symbols(Allocator), UsedNames(Allocator),
       InlineAsmUsedLabelNames(Allocator),
       CurrentDwarfLoc(0, 0, 0, DWARF2_FLAG_IS_STMT, 0, 0),
       AutoReset(DoAutoReset), TargetOptions(TargetOpts) {
-  SecureLogFile = AsSecureLogFileName;
+  SecureLogFile = TargetOptions ? TargetOptions->AsSecureLogFile : "";
 
   if (SrcMgr && SrcMgr->getNumBuffers())
     MainFileName = std::string(SrcMgr->getMemoryBuffer(SrcMgr->getMainFileID())
                                    ->getBufferIdentifier());
 
   switch (TheTriple.getObjectFormat()) {
   case Triple::MachO:
     Env = IsMachO;

llvm/lib/MC/MCParser/DarwinAsmParser.cpp

bool DarwinAsmParser::parseDirectiveSecureLogUnique(StringRef, SMLoc IDLoc) {
   StringRef LogMessage = getParser().parseStringToEndOfStatement();
   if (getLexer().isNot(AsmToken::EndOfStatement))
     return TokError("unexpected token in '.secure_log_unique' directive");
 
   if (getContext().getSecureLogUsed())
     return Error(IDLoc, ".secure_log_unique specified multiple times");
 
   // Get the secure log path.
-  const char *SecureLogFile = getContext().getSecureLogFile();
-  if (!SecureLogFile)
+  StringRef SecureLogFile = getContext().getSecureLogFile();
+  if (SecureLogFile.empty())
     return Error(IDLoc, ".secure_log_unique used but AS_SECURE_LOG_FILE "
                  "environment variable unset.");
 
   // Open the secure log file if we haven't already.
   raw_fd_ostream *OS = getContext().getSecureLog();
   if (!OS) {
     std::error_code EC;
-    auto NewOS = std::make_unique<raw_fd_ostream>(StringRef(SecureLogFile), EC,
-                                                  sys::fs::OF_Append |
-                                                      sys::fs::OF_TextWithCRLF);
+    auto NewOS = std::make_unique<raw_fd_ostream>(
+        SecureLogFile, EC, sys::fs::OF_Append | sys::fs::OF_TextWithCRLF);
     if (EC)
        return Error(IDLoc, Twine("can't open secure log file: ") +
                                SecureLogFile + " (" + EC.message() + ")");
     OS = NewOS.get();
     getContext().setSecureLog(std::move(NewOS));
   }
 
   // Write the message.

llvm/lib/MC/MCTargetOptionsCommandFlags.cpp

 MCOPT(bool, Dwarf64)
 MCOPT(EmitDwarfUnwindType, EmitDwarfUnwind)
 MCOPT(bool, ShowMCInst)
 MCOPT(bool, FatalWarnings)
 MCOPT(bool, NoWarn)
 MCOPT(bool, NoDeprecatedWarn)
 MCOPT(bool, NoTypeCheck)
 MCOPT(std::string, ABIName)
+MCOPT(std::string, AsSecureLogFile)
 
 llvm::mc::RegisterMCTargetOptionsFlags::RegisterMCTargetOptionsFlags() {
 #define MCBINDOPT(NAME)                                                        \
   do {                                                                         \
     NAME##View = std::addressof(NAME);                                         \
   } while (0)
 
   static cl::opt<bool> RelaxAll(
#define MCBINDOPT(NAME)                                                        \
   MCBINDOPT(NoTypeCheck);
 
   static cl::opt<std::string> ABIName(
       "target-abi", cl::Hidden,
       cl::desc("The name of the ABI to be targeted from the backend."),
       cl::init(""));
   MCBINDOPT(ABIName);
 
+  static cl::opt<std::string> AsSecureLogFile(
+      "as-secure-log-file", cl::desc("As secure log file name"), cl::Hidden);
+  MCBINDOPT(AsSecureLogFile);
+
 #undef MCBINDOPT
 }
 
 MCTargetOptions llvm::mc::InitMCTargetOptionsFromFlags() {
   MCTargetOptions Options;
   Options.MCRelaxAll = getRelaxAll();
   Options.MCIncrementalLinkerCompatible = getIncrementalLinkerCompatible();
   Options.Dwarf64 = getDwarf64();
   Options.DwarfVersion = getDwarfVersion();
   Options.ShowMCInst = getShowMCInst();
   Options.ABIName = getABIName();
   Options.MCFatalWarnings = getFatalWarnings();
   Options.MCNoWarn = getNoWarn();
   Options.MCNoDeprecatedWarn = getNoDeprecatedWarn();
   Options.MCNoTypeCheck = getNoTypeCheck();
   Options.EmitDwarfUnwind = getEmitDwarfUnwind();
+  Options.AsSecureLogFile = getAsSecureLogFile();
 
   return Options;
 }

llvm/test/MC/AsmParser/secure_log_unique.s

 // RUN: rm -f %t
-// RUN: env AS_SECURE_LOG_FILE=%t llvm-mc -triple x86_64-apple-darwin %s
-// RUN: env AS_SECURE_LOG_FILE=%t llvm-mc -triple x86_64-apple-darwin %s
+// RUN: llvm-mc -as-secure-log-file %t -triple x86_64-apple-darwin %s
+// RUN: llvm-mc -as-secure-log-file %t -triple x86_64-apple-darwin %s
 // RUN: FileCheck --input-file=%t %s
 .secure_log_unique "foobar"
 
 // CHECK: "foobar"
 // CHECK-NEXT: "foobar"