[Driver] Error for -gsplit-dwarf with RISC-V linker relaxation

[MaskRay]

Summary

-gsplit-dwarf produces a .dwo file which will not be processed by the linker. If
.dwo files contain relocations, they will not be resolved. Therefore the
practice is that .dwo files do not contain relocations.

Address ranges and location description need to use forms/entry kinds indexing
into .debug_addr (DW_FORM_addrx/DW_RLE_startx_endx/etc), which is currently not
implemented.

There is a difficult-to-read MC error with -gsplit-dwarf with RISC-V for both -mrelax and -mno-relax.

% clang --target=riscv64-linux-gnu -g -gsplit-dwarf -c a.c
error: A dwo section may not contain relocations

We expect to fix -mno-relax soon, so report a driver error for -mrelax for now.

Link: https://github.com/llvm/llvm-project/issues/56642

[MaskRay]

incompatible => unsupported

since it is implementable (but difficult).

[compnerd]

clang/lib/Driver/ToolChains/Arch/RISCV.cpp

10

This feels like a layering violation. I suppose that as long as modular builds are okay ...

clang/test/Driver/riscv-features.c

40

Did you mean to add -### to these two?

[MaskRay]

clang/lib/Driver/ToolChains/Arch/RISCV.cpp

10

lib/Driver/ToolChains/Arch does not form a module or library. Arch/ is just a subdirectory containing these arch-specific files. So I think this is fine.

clang/test/Driver/riscv-features.c

40

No. In the presence of errors, without -### the clang command exists with 1 and does not write to the output. I feel that this behavior is better for error testing.

[compnerd]

clang/lib/Driver/ToolChains/Arch/RISCV.cpp

146

This should be hoisted out, this will occur with or without relaxation. That is, -mrelax and -mno-relax are both problematic.

clang/test/Driver/riscv-features.c

41

Can you please add a -mno-relax check as well?

[MaskRay]

clang/lib/Driver/ToolChains/Arch/RISCV.cpp

146

No. -mno-relax is supported with -gsplit-dwarf. I think the current code is correct.

clang/test/Driver/riscv-features.c

41

The line you reference has -mno-relax :)

[kito-cheng]

LGTM

[compnerd]

clang/lib/Driver/ToolChains/Arch/RISCV.cpp

146

No, that is the point, it is not. Building with -mno-relax and -gsplit-dwarf will fail with the same behaviour.

[MaskRay]

clang/lib/Driver/ToolChains/Arch/RISCV.cpp

146

OK, I confirm. -mno-relax -gsplit-dwarf should be compatible and can be fixed separated (e.g. by your D130206, I suspect it doesn't cover all cases).

-mrelax -gsplit-dwarf will likely stay unsupported for a while so the driver error will not hurt.

clang/include/clang/Basic/DiagnosticDriverKinds.td

def err_drv_invalid_range_dxil_validator_version : Error<
   "invalid validator version : %0\n"
   "Validator version must be less than or equal to current internal version.">;
 def err_drv_invalid_format_dxil_validator_version : Error<
   "invalid validator version : %0\n"
   "Format of validator version is \"<major>.<minor>\" (ex:\"1.4\").">;
 def err_drv_invalid_empty_dxil_validator_version : Error<
   "invalid validator version : %0\n"
   "If validator major version is 0, minor version must also be 0.">;
+
+def err_drv_riscv_incompatible_with_linker_relaxation : Error<
+  "%0 is incompatible with RISC-V linker relaxation (-mrelax)">;
 }

clang/lib/Driver/ToolChains/Arch/RISCV.cpp

 //===--- RISCV.cpp - RISCV Helpers for Tools --------------------*- 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 "RISCV.h"
+#include "../Clang.h"
 #include "ToolChains/CommonArgs.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/Error.h"
void riscv::getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple,
   if (Args.hasArg(options::OPT_ffixed_x29))
     Features.push_back("+reserve-x29");
   if (Args.hasArg(options::OPT_ffixed_x30))
     Features.push_back("+reserve-x30");
   if (Args.hasArg(options::OPT_ffixed_x31))
     Features.push_back("+reserve-x31");
 
   // -mrelax is default, unless -mno-relax is specified.
-  if (Args.hasFlag(options::OPT_mrelax, options::OPT_mno_relax, true))
+  if (Args.hasFlag(options::OPT_mrelax, options::OPT_mno_relax, true)) {
     Features.push_back("+relax");
-  else
+    Arg *A;
+    if (getDebugFissionKind(D, Args, A) != DwarfFissionKind::None)
+      D.Diag(clang::diag::err_drv_riscv_incompatible_with_linker_relaxation)
+          << A->getAsString(Args);
+  } else {
     Features.push_back("-relax");
+  }
 
   // GCC Compatibility: -mno-save-restore is default, unless -msave-restore is
   // specified.
   if (Args.hasFlag(options::OPT_msave_restore, options::OPT_mno_save_restore, false))
     Features.push_back("+save-restore");
   else
     Features.push_back("-save-restore");
 

clang/lib/Driver/ToolChains/Clang.h

public:
 
   bool hasIntegratedCPP() const override { return false; }
   void ConstructJob(Compilation &C, const JobAction &JA,
                     const InputInfo &Output, const InputInfoList &Inputs,
                     const llvm::opt::ArgList &TCArgs,
                     const char *LinkingOutput) const override;
 };
 
+enum class DwarfFissionKind { None, Split, Single };
+
+DwarfFissionKind getDebugFissionKind(const Driver &D,
+                                     const llvm::opt::ArgList &Args,
+                                     llvm::opt::Arg *&Arg);
+
 } // end namespace tools
 
 } // end namespace driver
 } // end namespace clang
 
 #endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_CLANG_H

clang/lib/Driver/ToolChains/Clang.cpp

static void RenderDiagnosticsOptions(const Driver &D, const ArgList &Args,
   if (!Args.hasFlag(options::OPT_fshow_column, options::OPT_fno_show_column,
                     ColumnDefault))
     CmdArgs.push_back("-fno-show-column");
 
   Args.addOptOutFlag(CmdArgs, options::OPT_fspell_checking,
                      options::OPT_fno_spell_checking);
 }
 
-enum class DwarfFissionKind { None, Split, Single };
-
-static DwarfFissionKind getDebugFissionKind(const Driver &D,
+DwarfFissionKind tools::getDebugFissionKind(const Driver &D,
                                             const ArgList &Args, Arg *&Arg) {
   Arg = Args.getLastArg(options::OPT_gsplit_dwarf, options::OPT_gsplit_dwarf_EQ,
                         options::OPT_gno_split_dwarf);
   if (!Arg || Arg->getOption().matches(options::OPT_gno_split_dwarf))
     return DwarfFissionKind::None;
 
   if (Arg->getOption().matches(options::OPT_gsplit_dwarf))
     return DwarfFissionKind::Split;

clang/test/Driver/riscv-features.c

 // DEFAULT-LINUX-SAME: "-target-feature" "+a"
 // DEFAULT-LINUX-SAME: "-target-feature" "+f"
 // DEFAULT-LINUX-SAME: "-target-feature" "+d"
 // DEFAULT-LINUX-SAME: "-target-feature" "+c"
 
 // RUN: not %clang -cc1 -triple riscv64-unknown-elf -target-feature +e 2>&1 | FileCheck %s -check-prefix=RV64-WITH-E
 
 // RV64-WITH-E: error: invalid feature combination: standard user-level extension 'e' requires 'rv32'
+
+// RUN: not %clang -c --target=riscv64-linux-gnu -gsplit-dwarf %s 2>&1 | FileCheck %s --check-prefix=ERR-SPLIT-DWARF
+// RUN: not %clang -c --target=riscv64 -gsplit-dwarf=single %s 2>&1 | FileCheck %s --check-prefix=ERR-SPLIT-DWARF
+// RUN: %clang -### -c --target=riscv64 -mno-relax -g -gsplit-dwarf %s 2>&1 | FileCheck %s --check-prefix=SPLIT-DWARF
+
+// ERR-SPLIT-DWARF: error: -gsplit-dwarf{{.*}} is incompatible with RISC-V linker relaxation (-mrelax)
+// SPLIT-DWARF:     "-split-dwarf-file"