Skip to content

Guard against nullptr #94084

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

Sterling-Augustine
Copy link
Contributor

@Sterling-Augustine Sterling-Augustine commented Jun 1, 2024

Protect against nullptr after #93926

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 1, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 1, 2024

@llvm/pr-subscribers-clang

Author: None (Sterling-Augustine)

Changes

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

1 Files Affected:

  • (modified) clang/lib/AST/QualTypeNames.cpp (+3-2)
diff --git a/clang/lib/AST/QualTypeNames.cpp b/clang/lib/AST/QualTypeNames.cpp
index 066377423df76..18ac4b1eb57e7 100644
--- a/clang/lib/AST/QualTypeNames.cpp
+++ b/clang/lib/AST/QualTypeNames.cpp
@@ -65,8 +65,9 @@ static bool getFullyQualifiedTemplateName(const ASTContext &Ctx,
   assert(ArgTDecl != nullptr);
   QualifiedTemplateName *QTName = TName.getAsQualifiedTemplateName();
 
-  if (QTName && !QTName->hasTemplateKeyword()) {
-    NNS = QTName->getQualifier();
+  if (QTName &&
+      !QTName->hasTemplateKeyword() &&
+      (NNS = QTName->getQualifier())) {
     NestedNameSpecifier *QNNS = getFullyQualifiedNestedNameSpecifier(
         Ctx, NNS, WithGlobalNsPrefix);
     if (QNNS != NNS) {

Copy link

github-actions bot commented Jun 1, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff bba5ee47e63298d61f6ea441a140144ce370ba92 169e09f2aa403676873a440d8496356cefe69b9e -- clang/lib/AST/QualTypeNames.cpp
View the diff from clang-format here.
diff --git a/clang/lib/AST/QualTypeNames.cpp b/clang/lib/AST/QualTypeNames.cpp
index 18ac4b1eb5..737092b156 100644
--- a/clang/lib/AST/QualTypeNames.cpp
+++ b/clang/lib/AST/QualTypeNames.cpp
@@ -65,8 +65,7 @@ static bool getFullyQualifiedTemplateName(const ASTContext &Ctx,
   assert(ArgTDecl != nullptr);
   QualifiedTemplateName *QTName = TName.getAsQualifiedTemplateName();
 
-  if (QTName &&
-      !QTName->hasTemplateKeyword() &&
+  if (QTName && !QTName->hasTemplateKeyword() &&
       (NNS = QTName->getQualifier())) {
     NestedNameSpecifier *QNNS = getFullyQualifiedNestedNameSpecifier(
         Ctx, NNS, WithGlobalNsPrefix);

@Sterling-Augustine Sterling-Augustine merged commit 16832eb into llvm:main Jun 1, 2024
6 of 9 checks passed
@Sterling-Augustine Sterling-Augustine deleted the segfault_qualtype_name branch September 4, 2024 18:35
qtprojectorg pushed a commit to qt/qttools that referenced this pull request Sep 17, 2024
For compatibility reasons, QDoc carries a custom implementation of
`llvm-project.git/clang/lib/AST/QualTypeNames.cpp`. When QDoc is built
against Clang libraries from LLVM 19, a segmentation fault occurs when
generating the documentation for the Qt 3D module as part of a qt5.git
super-module documentation build.

The segmentation fault is the result of a `nullptr` being passed to
`clang::TypeName::getFullyQualifiedNestedNameSpecifier` for the
`Scope` parameter.

Upon investigation, it became clear that two changes have been made
upstream to the implementation QDoc carries a customized version of,
one of which adds a `nullptr` check. Due to the small footprint of
both changes, this patch applies both of them to QDoc's
`clang/AST/QualTypeNames.h`:

- The upstream change 16832eb58563f77d917198ad9f86db1c2ee162c9 adds a
  `nullptr` check, see llvm/llvm-project#94084
  for details.
- The upstream change 35bfbb3b21e9874d03b730e8ce4eb98b1dcd2d28
  replaces `dyn_cast_or_null<T>(foo)` with `dyn_cast<T>(foo)` for
  never-null arguments.

The changes apply also when QDoc is built against Clang libraries from
LLVM 17 and 18, with both end-to-end tests passing. Given the nature of
the changes, this means these adaptations do not require being wrapped
in `#if LIBCLANG_VERSION_MAJOR` checks.

Fixes: QTBUG-128926
Change-Id: I5863ca213a35042ed325971b42de2bc1e86c6457
Reviewed-by: Topi Reiniö <topi.reinio@qt.io>
qtprojectorg pushed a commit to qt/qttools that referenced this pull request Sep 18, 2024
For compatibility reasons, QDoc carries a custom implementation of
`llvm-project.git/clang/lib/AST/QualTypeNames.cpp`. When QDoc is built
against Clang libraries from LLVM 19, a segmentation fault occurs when
generating the documentation for the Qt 3D module as part of a qt5.git
super-module documentation build.

The segmentation fault is the result of a `nullptr` being passed to
`clang::TypeName::getFullyQualifiedNestedNameSpecifier` for the
`Scope` parameter.

Upon investigation, it became clear that two changes have been made
upstream to the implementation QDoc carries a customized version of,
one of which adds a `nullptr` check. Due to the small footprint of
both changes, this patch applies both of them to QDoc's
`clang/AST/QualTypeNames.h`:

- The upstream change 16832eb58563f77d917198ad9f86db1c2ee162c9 adds a
  `nullptr` check, see llvm/llvm-project#94084
  for details.
- The upstream change 35bfbb3b21e9874d03b730e8ce4eb98b1dcd2d28
  replaces `dyn_cast_or_null<T>(foo)` with `dyn_cast<T>(foo)` for
  never-null arguments. See
  llvm/llvm-project@35bfbb3
  for details.

The changes apply also when QDoc is built against Clang libraries from
LLVM 17 and 18, with both end-to-end tests passing. Given the nature of
the changes, this means these adaptations do not require being wrapped
in `#if LIBCLANG_VERSION_MAJOR` checks.

Fixes: QTBUG-128926
Pick-to: 6.8
Change-Id: I5863ca213a35042ed325971b42de2bc1e86c6457
Reviewed-by: Luca Di Sera <luca.disera@qt.io>
Reviewed-by: Topi Reiniö <topi.reinio@qt.io>
qtprojectorg pushed a commit to qt/qttools that referenced this pull request Sep 18, 2024
For compatibility reasons, QDoc carries a custom implementation of
`llvm-project.git/clang/lib/AST/QualTypeNames.cpp`. When QDoc is built
against Clang libraries from LLVM 19, a segmentation fault occurs when
generating the documentation for the Qt 3D module as part of a qt5.git
super-module documentation build.

The segmentation fault is the result of a `nullptr` being passed to
`clang::TypeName::getFullyQualifiedNestedNameSpecifier` for the
`Scope` parameter.

Upon investigation, it became clear that two changes have been made
upstream to the implementation QDoc carries a customized version of,
one of which adds a `nullptr` check. Due to the small footprint of
both changes, this patch applies both of them to QDoc's
`clang/AST/QualTypeNames.h`:

- The upstream change 16832eb58563f77d917198ad9f86db1c2ee162c9 adds a
  `nullptr` check, see llvm/llvm-project#94084
  for details.
- The upstream change 35bfbb3b21e9874d03b730e8ce4eb98b1dcd2d28
  replaces `dyn_cast_or_null<T>(foo)` with `dyn_cast<T>(foo)` for
  never-null arguments. See
  llvm/llvm-project@35bfbb3
  for details.

The changes apply also when QDoc is built against Clang libraries from
LLVM 17 and 18, with both end-to-end tests passing. Given the nature of
the changes, this means these adaptations do not require being wrapped
in `#if LIBCLANG_VERSION_MAJOR` checks.

Fixes: QTBUG-128926
Change-Id: I5863ca213a35042ed325971b42de2bc1e86c6457
Reviewed-by: Luca Di Sera <luca.disera@qt.io>
Reviewed-by: Topi Reiniö <topi.reinio@qt.io>
(cherry picked from commit a2f478b)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants