[clang] Fix a segfault when M is a nullptr #130712
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-modules Author: Matt (matts1) ChangesIf Full diff: https://github.com/llvm/llvm-project/pull/130712.diff 1 Files Affected:
diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index 62cc8acf9588b..3d035f0a5f787 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -888,7 +888,7 @@ class VisibleModuleSet {
/// Get the location at which the import of a module was triggered.
SourceLocation getImportLoc(const Module *M) const {
- return M->getVisibilityID() < ImportLocs.size()
+ return M && M->getVisibilityID() < ImportLocs.size()
? ImportLocs[M->getVisibilityID()]
: SourceLocation();
}
|
|
@llvm/pr-subscribers-clang Author: Matt (matts1) ChangesIf Full diff: https://github.com/llvm/llvm-project/pull/130712.diff 1 Files Affected:
diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index 62cc8acf9588b..3d035f0a5f787 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -888,7 +888,7 @@ class VisibleModuleSet {
/// Get the location at which the import of a module was triggered.
SourceLocation getImportLoc(const Module *M) const {
- return M->getVisibilityID() < ImportLocs.size()
+ return M && M->getVisibilityID() < ImportLocs.size()
? ImportLocs[M->getVisibilityID()]
: SourceLocation();
}
|
|
There is a bug report on the chromium bug tracker - http://crbug.com/400353616 |
matts1
force-pushed
the
push-qwyquzltunws
branch
from
4b41cf9 to
cf5684b
Compare
|
I have a repro, but it may be addressing the symptoms rather than the root cause. AFAICT, a ModuleMacro should always have an associated module. The root cause of the issue appears to be that in the example I provided with the test case, the I'm happy to submit this as is, since a header guard can't hurt, but it may just hide the problem. |
clang/test/Modules/pr130712.cppm
Outdated
| // 1) There must be a configuration mismatch between a header and a file it depends on | ||
| // 2) -fmodules-local-submodule-visibility must be enabled. | ||
|
|
||
| // Unfortunately, libcxx is compiled with exceptions, but the sysroot it depends on is likely compiled without exceptions. |
There was a problem hiding this comment.
Why libcxx is relevant here while any code in this test doesn't include STL header?
There was a problem hiding this comment.
You can reproduce this with libcxx, but this is a minimal repro.
There was a problem hiding this comment.
The comment is not relevant to the test; can you remove it?
matts1
force-pushed
the
push-qwyquzltunws
branch
from
cf5684b to
9d29127
Compare
clang/test/Modules/pr130712.cppm
Outdated
| // 1) There must be a configuration mismatch between a header and a file it depends on | ||
| // 2) -fmodules-local-submodule-visibility must be enabled. | ||
|
|
||
| // Unfortunately, libcxx is compiled with exceptions, but the sysroot it depends on is likely compiled without exceptions. |
There was a problem hiding this comment.
The comment is not relevant to the test; can you remove it?
matts1
force-pushed
the
push-qwyquzltunws
branch
from
76fc1c9 to
2c0dc1e
Compare
matts1
force-pushed
the
push-qwyquzltunws
branch
from
a249bae to
195055e
Compare
If
MM->getOwningModulereturns nullptr, thenisVisibleis called with nullptr, which then callsgetImportLoc(nullptr)llvm-project/clang/lib/Lex/PPMacroExpansion.cpp
Line 208 in 077e0c1