[AVR] Automatically link CRT and libgcc from the system avr-gcc

[dylanmckay]

Summary

This patch modifies the AVR toolchain so that if avr-gcc and avr-libc
are detected during compilation, the CRT, libgcc, libm, and libc anre
linked.

This matches avr-gcc's default behaviour, and the expected behaviour of
all C compilers - including the C runtime.

avr-gcc also needs a -mmcu specified in order to link runtime libraries.

The difference betwen this patch and avr-gcc is that this patch will
warn users whenever they compile without a runtime, as opposed to GCC,
which silently trims the runtime libs from the linker arguments when no
-mmcu is specified.

[dylanmckay]

Add the search path that Ubuntu installs libc to.

[dylanmckay]

I'd like feedback on the new AVR-specific warnings, and the new warning group. I have not added a warning or error to clang before, but this seems consistent with how the other targets implement it.

[aaron.ballman]

include/clang/Basic/DiagnosticDriverKinds.td

43–44

This is novel for diagnostics -- we don't have any other diagnostics that recommend filing bugs. This doesn't strike me as something a naive developer will just happen across by accident, so I would probably drop from the comma onward and assume the user knows they can report bugs if they really care.

lib/Driver/ToolChains/AVR.cpp

39

Why use std::string here when below you use it as a list of StringRefs?

91–93

Elide braces.

128

I'd drop this and inline it in the use below as *Family.

154–156

Elide braces.

159

return llvm::None;

lib/Driver/ToolChains/AVR.h

45–49

Did clang-format produce this? For some reason, it looks off to my eyes.

[dylanmckay]

• Remove link to BugZilla in diagnostic
• Use StringRef for a static string array rather than std::string
• Elide braces
• Dereference an Optional in-place rather than persisting it for one use
• Use llvm::None rather than explicit Optional() constructor

[dylanmckay]

Run clang-format on the whole patch

[dylanmckay]

include/clang/Basic/DiagnosticDriverKinds.td

43–44

I agree

lib/Driver/ToolChains/AVR.cpp

39

Good point, have fixed.

159

Didn't know about that, thanks!

lib/Driver/ToolChains/AVR.h

45–49

I have now run clang-format on the whole patch.

[aaron.ballman]

I'm not certain if it will be possible to devise test cases for the two diagnostics I pointed out or not, but otherwise, this LGTM as far as the implementation goes. I don't know enough about AVR to sign off on whether the patch logic is correct or not.

include/clang/Basic/DiagnosticDriverKinds.td

34

I don't see a test case for this diagnostic.

39

Or this one.

[dylanmckay]

I'm not certain if it will be possible to devise test cases for the two diagnostics I pointed out or not

I don't think it will be. The compile warning logic works by directly querying the physical filesystem using the llvm::sys::fs APIs, which don't appear to support mocking or virtual filesystems. The only way for the pragmatically trigger the warnings is to dangerously manipulate the filesystem, potentially destroying the user's avr-gcc installation.

I don't know enough about AVR to sign off on whether the patch logic is correct or not.

I've added some review comments to the AVR logic referencing the GCC manual and how this patch matches it. I've also added the only two LLVM reviewers I know have AVR experience so hopefully they will be able to comment on the accuracy of the AVR logic.

lib/Driver/ToolChains/AVR.cpp

34

Here is the avr-gcc list of devices and their architectures

atmega328 and atmega328p are a part of the avr5 family.

avr5

“Enhanced” devices with 16 KiB up to 64 KiB of program memory.
mcu = ata5702m322, ata5782, ata5790, ata5790n, ata5791, ata5795, ata5831, ata6613c, ata6614q, ata8210, ata8510, atmega16, atmega16a, atmega16hva, atmega16hva2, atmega16hvb, atmega16hvbrevb, atmega16m1, atmega16u4, atmega161, atmega162, atmega163, atmega164a, atmega164p, atmega164pa, atmega165, atmega165a, atmega165p, atmega165pa, atmega168, atmega168a, atmega168p, atmega168pa, atmega168pb, atmega169, atmega169a, atmega169p, atmega169pa, atmega32, atmega32a, atmega32c1, atmega32hvb, atmega32hvbrevb, atmega32m1, atmega32u4, atmega32u6, atmega323, atmega324a, atmega324p, atmega324pa, atmega325, atmega325a, atmega325p, atmega325pa, atmega3250, atmega3250a, atmega3250p, atmega3250pa, atmega328, atmega328p, atmega328pb, atmega329, atmega329a, atmega329p, atmega329pa, atmega3290, atmega3290a, atmega3290p, atmega3290pa, atmega406, atmega64, atmega64a, atmega64c1, atmega64hve, atmega64hve2, atmega64m1, atmega64rfr2, atmega640, atmega644, atmega644a, atmega644p, atmega644pa, atmega644rfr2, atmega645, atmega645a, atmega645p, atmega6450, atmega6450a, atmega6450p, atmega649, atmega649a, atmega649p, atmega6490, atmega6490a, atmega6490p, at90can32, at90can64, at90pwm161, at90pwm216, at90pwm316, at90scr100, at90usb646, at90usb647, at94k, m3000.

https://gcc.gnu.org/onlinedocs/gcc/AVR-Options.html

138

Here is the avr-gcc manual documenting the filename of this particular libary[1].

Options:

-nodevicelib

Don’t link against AVR-LibC’s device specific library lib<mcu>.a.

This patch follows the same conventions as avr-gcc and thus can link the same libraries as avr-gcc.

[1] - https://gcc.gnu.org/onlinedocs/gcc/AVR-Options.html

142

This is an existing piece of avr-gcc logic also implemented by the LLVM AVR backend

-mmcu=mcu

Specify Atmel AVR instruction set architectures (ISA) or MCU type.

The default for this option is ‘avr2’.

https://gcc.gnu.org/onlinedocs/gcc/AVR-Options.html

[dylanmckay]

I'm very comfortable with the AVR changes at this point, I am going to go ahead and commit the patch.

include/clang/Basic/DiagnosticDriverKinds.td

def err_drv_unknown_stdin_type_clang_cl : Error<
   "use /Tc or /Tp to set input type for standard input">;
 def err_drv_unknown_language : Error<"language not recognized: '%0'">;
 def err_drv_invalid_arch_name : Error<
   "invalid arch name '%0'">;
 def err_drv_invalid_riscv_arch_name : Error<
   "invalid arch name '%0', %1">;
 def err_drv_invalid_riscv_ext_arch_name : Error<
   "invalid arch name '%0', %1 '%2'">;
+def warn_drv_avr_mcu_not_specified : Warning<
+  "no target microcontroller specified on command line, cannot "
+  "link standard libraries, please pass -mmcu=<mcu name>">,
+  InGroup<AVRRtlibLinkingQuirks>;
+def warn_drv_avr_gcc_not_found: Warning<
+  "no avr-gcc installation can be found on the system, "
+  "cannot link standard libraries">,
+  InGroup<AVRRtlibLinkingQuirks>;
+def warn_drv_avr_libc_not_found: Warning<
+  "no avr-libc installation can be found on the system, "
+  "cannot link standard libraries">,
+  InGroup<AVRRtlibLinkingQuirks>;
+def warn_drv_avr_family_linking_stdlibs_not_implemented: Warning<
+  "support for linking stdlibs for microcontroller '%0' is not implemented">,
+  InGroup<AVRRtlibLinkingQuirks>;
+def warn_drv_avr_stdlib_not_linked: Warning<
+  "standard library not linked and so no interrupt vector table or "
+  "compiler runtime routines will be linked">,
+  InGroup<AVRRtlibLinkingQuirks>;
 def err_drv_cuda_bad_gpu_arch : Error<"Unsupported CUDA gpu architecture: %0">;
 def err_drv_no_cuda_installation : Error<
   "cannot find CUDA installation.  Provide its path via --cuda-path, or pass "
   "-nocudainc to build without CUDA includes.">;
 def err_drv_no_cuda_libdevice : Error<
   "cannot find libdevice for %0. Provide path to different CUDA installation "
   "via --cuda-path, or pass -nocudalib to build without linking with libdevice.">;
 def err_drv_cuda_version_unsupported : Error<

include/clang/Basic/DiagnosticGroups.td

 
 // Issues with serialized diagnostics.
 def SerializedDiagnostics : DiagGroup<"serialized-diagnostics">;
 
 // A warning group for warnings about code that clang accepts when
 // compiling CUDA C/C++ but which is not compatible with the CUDA spec.
 def CudaCompat : DiagGroup<"cuda-compat">;
 
+// Warnings which cause linking of the runtime libraries like
+// libc and the CRT to be skipped.
+def AVRRtlibLinkingQuirks : DiagGroup<"avr-rtlib-linking-quirks">;
+
 // A warning group for things that will change semantics in the future.
 def FutureCompat : DiagGroup<"future-compat">;
 
 def InvalidOrNonExistentDirectory : DiagGroup<"invalid-or-nonexistent-directory">;
 
 def OptionIgnored : DiagGroup<"option-ignored">;
 
 def UnknownArgument : DiagGroup<"unknown-argument">;

lib/Driver/ToolChains/AVR.h

 #include "clang/Driver/ToolChain.h"
 #include "clang/Driver/Tool.h"
 
 namespace clang {
 namespace driver {
 namespace toolchains {
 
 class LLVM_LIBRARY_VISIBILITY AVRToolChain : public Generic_ELF {
-protected:
-  Tool *buildLinker() const override;
 public:
   AVRToolChain(const Driver &D, const llvm::Triple &Triple,
                const llvm::opt::ArgList &Args);
+
+protected:
+  Tool *buildLinker() const override;
+
+private:
+  /// Whether libgcc, libct, and friends should be linked.
+  ///
+  /// This is not done if the user does not specify a
+  /// microcontroller on the command line.
+  bool LinkStdlib;
+
+  llvm::Optional<std::string> findAVRLibcInstallation() const;
 };
 
 } // end namespace toolchains
 
 namespace tools {
 namespace AVR {
 class LLVM_LIBRARY_VISIBILITY Linker : public GnuTool {
 public:
-  Linker(const ToolChain &TC) : GnuTool("AVR::Linker", "avr-ld", TC) {}
+  Linker(const llvm::Triple &Triple, const ToolChain &TC, bool LinkStdlib)
+      : GnuTool("AVR::Linker", "avr-ld", TC), Triple(Triple),
+        LinkStdlib(LinkStdlib) {}
+
   bool hasIntegratedCPP() const override { return false; }
   bool isLinkJob() const override { return true; }
   void ConstructJob(Compilation &C, const JobAction &JA,
                     const InputInfo &Output, const InputInfoList &Inputs,
                     const llvm::opt::ArgList &TCArgs,
                     const char *LinkingOutput) const override;
+
+protected:
+  const llvm::Triple &Triple;
+  bool LinkStdlib;
 };
 } // end namespace AVR
 } // end namespace tools
 } // end namespace driver
 } // end namespace clang
 
 #endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_AVR_H

lib/Driver/ToolChains/AVR.cpp

 //===--- AVR.cpp - AVR ToolChain Implementations ----------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "AVR.h"
 #include "CommonArgs.h"
 #include "InputInfo.h"
 #include "clang/Driver/Compilation.h"
+#include "clang/Driver/DriverDiagnostic.h"
+#include "clang/Driver/Options.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/MC/MCSubtargetInfo.h"
+#include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/FileSystem.h"
 
 using namespace clang::driver;
 using namespace clang::driver::toolchains;
 using namespace clang::driver::tools;
 using namespace clang;
 using namespace llvm::opt;
 
+namespace {
+
+// TODO: Consider merging this into the AVR device table
+// array in Targets/AVR.cpp.
+llvm::Optional<StringRef> GetMcuFamilyName(StringRef MCU) {
+  return llvm::StringSwitch<llvm::Optional<StringRef>>(MCU)
+      .Case("atmega328", Optional<StringRef>("avr5"))
+      .Case("atmega328p", Optional<StringRef>("avr5"))
+      .Default(Optional<StringRef>());
+}
+
+const StringRef PossibleAVRLibcLocations[] = {
+    "/usr/avr",
+    "/usr/lib/avr",
+};
+
+} // end anonymous namespace
+
 /// AVR Toolchain
 AVRToolChain::AVRToolChain(const Driver &D, const llvm::Triple &Triple,
                            const ArgList &Args)
-  : Generic_ELF(D, Triple, Args) { }
+    : Generic_ELF(D, Triple, Args), LinkStdlib(false) {
+  GCCInstallation.init(Triple, Args);
+
+  // Only add default libraries if the user hasn't explicitly opted out.
+  if (!Args.hasArg(options::OPT_nostdlib) &&
+      !Args.hasArg(options::OPT_nodefaultlibs) &&
+      !Args.hasArg(options::OPT_c /* does not apply when not linking */)) {
+    std::string CPU = getCPUName(Args, Triple);
+
+    if (CPU.empty()) {
+      // We cannot link any standard libraries without an MCU specified.
+      D.Diag(diag::warn_drv_avr_mcu_not_specified);
+    } else {
+      Optional<StringRef> FamilyName = GetMcuFamilyName(CPU);
+      Optional<std::string> AVRLibcRoot = findAVRLibcInstallation();
+
+      if (!FamilyName.hasValue()) {
+        // We do not have an entry for this CPU in the family
+        // mapping table yet.
+        D.Diag(diag::warn_drv_avr_family_linking_stdlibs_not_implemented)
+            << CPU;
+      } else if (!GCCInstallation.isValid()) {
+        // No avr-gcc found and so no runtime linked.
+        D.Diag(diag::warn_drv_avr_gcc_not_found);
+      } else if (!AVRLibcRoot.hasValue()) {
+        // No avr-libc found and so no runtime linked.
+        D.Diag(diag::warn_drv_avr_libc_not_found);
+      } else { // We have enough information to link stdlibs
+        std::string GCCRoot = GCCInstallation.getInstallPath();
+        std::string LibcRoot = AVRLibcRoot.getValue();
+
+        getFilePaths().push_back(LibcRoot + std::string("/lib/") +
+                                 std::string(*FamilyName));
+        getFilePaths().push_back(LibcRoot + std::string("/lib/") +
+                                 std::string(*FamilyName));
+        getFilePaths().push_back(GCCRoot + std::string("/") +
+                                 std::string(*FamilyName));
+
+        LinkStdlib = true;
+      }
+    }
+
+    if (!LinkStdlib)
+      D.Diag(diag::warn_drv_avr_stdlib_not_linked);
+  }
+}
+
 Tool *AVRToolChain::buildLinker() const {
-  return new tools::AVR::Linker(*this);
+  return new tools::AVR::Linker(getTriple(), *this, LinkStdlib);
 }
 
 void AVR::Linker::ConstructJob(Compilation &C, const JobAction &JA,
                                const InputInfo &Output,
                                const InputInfoList &Inputs,
                                const ArgList &Args,
                                const char *LinkingOutput) const {
+  // Compute information about the target AVR.
+  std::string CPU = getCPUName(Args, getToolChain().getTriple());
+  llvm::Optional<StringRef> FamilyName = GetMcuFamilyName(CPU);
 
   std::string Linker = getToolChain().GetProgramPath(getShortName());
   ArgStringList CmdArgs;
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
+
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
+
+  // Enable garbage collection of unused sections.
+  CmdArgs.push_back("--gc-sections");
+
+  // Add library search paths before we specify libraries.
+  Args.AddAllArgs(CmdArgs, options::OPT_L);
+  getToolChain().AddFilePathLibArgs(Args, CmdArgs);
+
+  // If the family name is known, we can link with the device-specific libgcc.
+  // Without it, libgcc will simply not be linked. This matches avr-gcc
+  // behavior.
+  if (LinkStdlib) {
+    assert(!CPU.empty() && "CPU name must be known in order to link stdlibs");
+
+    // Add the object file for the CRT.
+    std::string CrtFileName = std::string("-l:crt") + CPU + std::string(".o");
+    CmdArgs.push_back(Args.MakeArgString(CrtFileName));
+
+    CmdArgs.push_back("-lgcc");
+    CmdArgs.push_back("-lm");
+    CmdArgs.push_back("-lc");
+
+    // Add the link library specific to the MCU.
+    CmdArgs.push_back(Args.MakeArgString(std::string("-l") + CPU));
+
+    // Specify the family name as the emulation mode to use.
+    // This is almost always required because otherwise avr-ld
+    // will assume 'avr2' and warn about the program being larger
+    // than the bare minimum supports.
+    CmdArgs.push_back(Args.MakeArgString(std::string("-m") + *FamilyName));
+  }
+
   C.addCommand(llvm::make_unique<Command>(JA, *this, Args.MakeArgString(Linker),
                                           CmdArgs, Inputs));
 }
-// AVR tools end.
+
+llvm::Optional<std::string> AVRToolChain::findAVRLibcInstallation() const {
+  for (StringRef PossiblePath : PossibleAVRLibcLocations) {
+    // Return the first avr-libc installation that exists.
+    if (llvm::sys::fs::is_directory(PossiblePath))
+      return Optional<std::string>(std::string(PossiblePath));
+  }
+
+  return llvm::None;
+}

lib/Driver/ToolChains/Gnu.cpp

static const char *const ARMHFTriples[] = {"arm-linux-gnueabihf",
                                              "armv6hl-suse-linux-gnueabi",
                                              "armv7hl-suse-linux-gnueabi"};
   static const char *const ARMebLibDirs[] = {"/lib"};
   static const char *const ARMebTriples[] = {"armeb-linux-gnueabi",
                                              "armeb-linux-androideabi"};
   static const char *const ARMebHFTriples[] = {
       "armeb-linux-gnueabihf", "armebv7hl-redhat-linux-gnueabi"};
 
+  static const char *const AVRLibDirs[] = {"/lib"};
+  static const char *const AVRTriples[] = {"avr"};
+
   static const char *const X86_64LibDirs[] = {"/lib64", "/lib"};
   static const char *const X86_64Triples[] = {
       "x86_64-linux-gnu",       "x86_64-unknown-linux-gnu",
       "x86_64-pc-linux-gnu",    "x86_64-redhat-linux6E",
       "x86_64-redhat-linux",    "x86_64-suse-linux",
       "x86_64-manbo-linux-gnu", "x86_64-linux-gnu",
       "x86_64-slackware-linux", "x86_64-unknown-linux",
       "x86_64-amazon-linux",    "x86_64-linux-android"};
/*static*/ void Generic_GCC::GCCInstallationDetector::CollectLibDirsAndTriples(
   case llvm::Triple::thumbeb:
     LibDirs.append(begin(ARMebLibDirs), end(ARMebLibDirs));
     if (TargetTriple.getEnvironment() == llvm::Triple::GNUEABIHF) {
       TripleAliases.append(begin(ARMebHFTriples), end(ARMebHFTriples));
     } else {
       TripleAliases.append(begin(ARMebTriples), end(ARMebTriples));
     }
     break;
+  case llvm::Triple::avr:
+    LibDirs.append(begin(AVRLibDirs), end(AVRLibDirs));
+    TripleAliases.append(begin(AVRTriples), end(AVRTriples));
+    break;
   case llvm::Triple::x86_64:
     LibDirs.append(begin(X86_64LibDirs), end(X86_64LibDirs));
     TripleAliases.append(begin(X86_64Triples), end(X86_64Triples));
     // x32 is always available when x86_64 is available, so adding it as
     // secondary arch with x86_64 triples
     if (TargetTriple.getEnvironment() == llvm::Triple::GNUX32) {
       BiarchLibDirs.append(begin(X32LibDirs), end(X32LibDirs));
       BiarchTripleAliases.append(begin(X86_64Triples), end(X86_64Triples));
if (isArmOrThumbArch(TargetArch) && TargetTriple.isAndroid()) {
     findAndroidArmMultilibs(D, TargetTriple, Path, Args, Detected);
   } else if (TargetTriple.isMIPS()) {
     if (!findMIPSMultilibs(D, TargetTriple, Path, Args, Detected))
       return false;
   } else if (isRISCV(TargetArch)) {
     findRISCVMultilibs(D, TargetTriple, Path, Args, Detected);
   } else if (isMSP430(TargetArch)) {
     findMSP430Multilibs(D, TargetTriple, Path, Args, Detected);
+  } else if (TargetArch == llvm::Triple::avr) {
+    // AVR has no multilibs.
   } else if (!findBiarchMultilibs(D, TargetTriple, Path, Args,
                                   NeedsBiarchSuffix, Detected)) {
     return false;
   }
 
   Multilibs = Detected.Multilibs;
   SelectedMultilib = Detected.SelectedMultilib;
   BiarchSibling = Detected.BiarchSibling;

test/Driver/avr-link-mcu-family-unimplemented.c

+// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps -mmcu=attiny13a %s 2>&1 | FileCheck --check-prefix=WARN %s
+
+// WARN: warning: support for linking stdlibs for microcontroller 'attiny13a' is not implemented
+// WARN: warning: standard library not linked and so no interrupt vector table or compiler runtime routines will be linked
+
+int main() { return 0; }
+

test/Driver/avr-link-no-mcu-specified.c

+// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps %s 2>&1 | FileCheck --check-prefix=WARN %s
+// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps -mmcu=atmega328 %s 2>&1 | FileCheck --check-prefix=NOWARN %s
+
+// WARN: warning: no target microcontroller specified on command line, cannot link standard libraries, please pass -mmcu=<mcu name>
+// WARN: warning: standard library not linked and so no interrupt vector table or compiler runtime routines will be linked
+
+// NOWARN: main
+
+int main() { return 0; }
+

test/Driver/avr-link-nostdlib-nodefaultlibs.c

+// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps -mmcu=atmega328 -nostdlib %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps -mmcu=atmega328 -nodefaultlibs %s 2>&1 | FileCheck %s
+
+// nostdlib and nodefaultlibs programs should compile fine.
+
+// CHECK: main
+int main() { return 0; }
+