Skip to content

Conversation

alanzhao1
Copy link
Contributor

Clang vendors may need to set a custom message for bug reports if the default is not sufficient (e.g. if they need the user to provide more information than what the default message asks for). This can be configured using the new macro CLANG_CUSTOM_BUG_REPORT_MSG.

Clang vendors may need to set a custom message for bug reports if the
default is not sufficient (e.g. if they need the user to provide more
information than what the default message asks for). This can be
configured using the new macro `CLANG_CUSTOM_BUG_REPORT_MSG`.
@alanzhao1 alanzhao1 requested a review from rnk September 8, 2025 19:47
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Sep 8, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 8, 2025

@llvm/pr-subscribers-clang

Author: Alan Zhao (alanzhao1)

Changes

Clang vendors may need to set a custom message for bug reports if the default is not sufficient (e.g. if they need the user to provide more information than what the default message asks for). This can be configured using the new macro CLANG_CUSTOM_BUG_REPORT_MSG.


Full diff: https://github.com/llvm/llvm-project/pull/157536.diff

2 Files Affected:

  • (modified) clang/include/clang/Config/config.h.cmake (+3)
  • (modified) clang/tools/driver/driver.cpp (+4)
diff --git a/clang/include/clang/Config/config.h.cmake b/clang/include/clang/Config/config.h.cmake
index 00c352b458c34..4194c00c756c4 100644
--- a/clang/include/clang/Config/config.h.cmake
+++ b/clang/include/clang/Config/config.h.cmake
@@ -85,4 +85,7 @@
 /* Whether CIR is built into Clang */
 #cmakedefine01 CLANG_ENABLE_CIR
 
+/* Define if there is a custom bug report message */
+#cmakedefine CLANG_CUSTOM_BUG_REPORT_MSG "${CLANG_CUSTOM_BUG_REPORT_MSG}"
+
 #endif
diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp
index e5c3c4ed5f804..c604d2927c869 100644
--- a/clang/tools/driver/driver.cpp
+++ b/clang/tools/driver/driver.cpp
@@ -235,9 +235,13 @@ static int ExecuteCC1Tool(SmallVectorImpl<const char *> &ArgV,
 
 int clang_main(int Argc, char **Argv, const llvm::ToolContext &ToolContext) {
   noteBottomOfStack();
+  #ifdef CLANG_CUSTOM_BUG_REPORT_MSG
+  llvm::setBugReportMsg(CLANG_CUSTOM_BUG_REPORT_MSG);
+  #else
   llvm::setBugReportMsg("PLEASE submit a bug report to " BUG_REPORT_URL
                         " and include the crash backtrace, preprocessed "
                         "source, and associated run script.\n");
+  #endif
   SmallVector<const char *, 256> Args(Argv, Argv + Argc);
 
   if (llvm::sys::Process::FixupStandardFileDescriptors())

Copy link

github-actions bot commented Sep 8, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@rnk
Copy link
Collaborator

rnk commented Sep 9, 2025

I cc'd some folks who are invested in the CMake and Bazel build configurations. How do you all (@petrhosek , @slackito , and @chapuni ) think this should be modeled? Does this cmakedefine transfer over easily to the Bazel build?

@slackito slackito requested a review from rupprecht September 9, 2025 19:24
@slackito
Copy link
Collaborator

slackito commented Sep 9, 2025

AFAICT for the bazel build we'd need to update this manually-ported clang/include/clang/Config/config.h: https://github.com/llvm/llvm-project/blob/main/utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

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

I think this is sensible, but would like to hear from a few downstreams what their requirements for something like this would be. It would be nice if we could come up with something flexible enough for everone and not have to revisit this.

@Endilll
Copy link
Contributor

Endilll commented Sep 10, 2025

I agree with Erich that it would be nice this to be a downstream-driven effort if it's not already.

@alanzhao1
Copy link
Contributor Author

I spoke with @rnk offline and he suggested that based on the previous two comments we should track this discussion in a bug, so I created #158203.

In the meantime I'll close this PR to put it on hold.

@alanzhao1 alanzhao1 closed this Sep 12, 2025
@alanzhao1 alanzhao1 reopened this Sep 15, 2025
@alanzhao1 alanzhao1 marked this pull request as draft September 15, 2025 18:29
@alanzhao1
Copy link
Contributor Author

In the meantime I'll close this PR to put it on hold.

It seems like the correct thing to do in this case is to convert this PR to a draft.

@rnk
Copy link
Collaborator

rnk commented Oct 8, 2025

@llvm/clang-vendors -- We're looking for additional input on the best way to customize the clang crash diagnostic. The question to downstream vendors is, is this interface (a cmake defined macro) sufficient flexibility, or is there a better way to enable this customization, or are our needs so different that we should all manage our own local downstream patches?

In particular, at Google, where we make heavy use of clang header files, collecting a single crash reproducer, even with PCM files, is not a good use of customer time. When we need to reduce a compiler crash, typically our first step is to rule out modules by replaying the build with modules disabled, which is doable, since we use clang header modules, not C++20 modules. If we can get single-file pre-processed source, that is preferred, since cvise works on it, and will go faster than it does working over a multi-module reproduction script. CVise does have support for module reducers thanks to @emaxx-google 's efforts, but even still, ruling out modules is generally a good first step.

@efriedma-quic
Copy link
Collaborator

Qualcomm currently has an internal patch that changes the string. I think this proposal is sufficient for our usage.

Note that there are a couple of clang regression tests that check for the "PLEASE submit a bug report" text; any patch here also needs to update those tests.

@alanzhao1
Copy link
Contributor Author

Note that there are a couple of clang regression tests that check for the "PLEASE submit a bug report" text; any patch here also needs to update those tests.

Unless I'm missing something this patch as is should be fine - it only changes the text if the macro is set.

@efriedma-quic
Copy link
Collaborator

It won't break regression tests on any of the buildbots, sure, but it will break regression tests if anyone actually uses the CMake flag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants