OikawaKirie created this revision.
OikawaKirie added reviewers: aaron.ballman, avl, mehdi_amini, ilya-biryukov,
tejohnson, jansvoboda11.
OikawaKirie added projects: LLVM, clang.
Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, steven_wu,
hiraditya.
OikawaKirie requested review of this revision.
All these potential null pointer dereferences are reported by my static
analyzer for null smart pointer dereferences, which has a different
implementation from `alpha.cplusplus.SmartPtr`.
The checked pointers are:
- The return value of `createArgument` in file
clang/utils/TableGen/ClangAttrEmitter.cpp. Although there are a lot of checks
in the function, nullptr is still allowed to be returned. As a recursive
function it is, I added checks to all the places where the function is called.
- The local variable `Unit` in function `DWARFLinker::loadClangModule` in file
llvm/lib/DWARFLinker/DWARFLinker.cpp. If the variable is not set in the loop
below its definition, it will trigger a null pointer dereference after the loop.
- The local variable `Index` in function `ThinLTOCodeGenerator::run` in file
llvm/lib/LTO/ThinLTOCodeGenerator.cpp. When function
`ThinLTOCodeGenerator::linkCombinedIndex` returns nullptr, the pointer `Index`
will be null and be dereferenced below.
- The parameter variable `Buffer` in function `InMemoryFileSystem::addFile` in
file llvm/lib/Support/VirtualFileSystem.cpp. The assertion in this function
(`assert(!(HardLinkTarget && Buffer))`) only checks whether these two
parameters can both be non-null. But It can be inferred that both pointers can
be null together. A null `Buffer` pointer can be dereferenced without a check.
- The return value of function `ModuleLazyLoaderCache::operator` in file
llvm/tools/llvm-link/llvm-link.cpp. According to the bug report of my static
analyzer, the std::function variable `ModuleLazyLoaderCache::createLazyModule`
points to function `loadFile`, which may return nullptr when error. And the
pointer is returned as a reference without a check to the return value.
- The local variable `Ret` in function `MarshallingKindInfo::create` in file
`llvm/utils/TableGen/OptParserEmitter.cpp`. If not all MarshallingKind's are
handled, variable `Ret` will be kept as nullptr.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D91844
Files:
clang/utils/TableGen/ClangAttrEmitter.cpp
llvm/lib/DWARFLinker/DWARFLinker.cpp
llvm/lib/LTO/ThinLTOCodeGenerator.cpp
llvm/lib/Support/VirtualFileSystem.cpp
llvm/tools/llvm-link/llvm-link.cpp
llvm/utils/TableGen/OptParserEmitter.cpp
Index: llvm/utils/TableGen/OptParserEmitter.cpp
===================================================================
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -183,6 +183,7 @@
Ret = std::make_unique<MarshallingInfoBooleanFlag>(
R, *R.getValueAsDef("NegOption"));
}
+ assert(Ret && "Unknown MarshallingInfoKind.");
Ret->ShouldAlwaysEmit = R.getValueAsBit("ShouldAlwaysEmit");
Ret->KeyPath = R.getValueAsString("KeyPath");
Index: llvm/tools/llvm-link/llvm-link.cpp
===================================================================
--- llvm/tools/llvm-link/llvm-link.cpp
+++ llvm/tools/llvm-link/llvm-link.cpp
@@ -243,8 +243,10 @@
Module &ModuleLazyLoaderCache::operator()(const char *argv0,
const std::string &Identifier) {
auto &Module = ModuleMap[Identifier];
- if (!Module)
+ if (!Module) {
Module = createLazyModule(argv0, Identifier);
+ assert(Module && "Failed to create lazy module!");
+ }
return *Module;
}
} // anonymous namespace
Index: llvm/lib/Support/VirtualFileSystem.cpp
===================================================================
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -732,6 +732,7 @@
if (HardLinkTarget)
Child.reset(new detail::InMemoryHardLink(P.str(), *HardLinkTarget));
else {
+ assert(Buffer && "Non-HardLink without a buffer?");
// Create a new file or directory.
Status Stat(P.str(), getNextVirtualUniqueID(),
llvm::sys::toTimePoint(ModificationTime), ResolvedUser,
@@ -771,6 +772,7 @@
return false;
// Return false only if the new file is different from the existing one.
+ assert(Buffer);
if (auto Link = dyn_cast<detail::InMemoryHardLink>(Node)) {
return Link->getResolvedFile().getBuffer()->getBuffer() ==
Buffer->getBuffer();
Index: llvm/lib/LTO/ThinLTOCodeGenerator.cpp
===================================================================
--- llvm/lib/LTO/ThinLTOCodeGenerator.cpp
+++ llvm/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -959,6 +959,7 @@
// Sequential linking phase
auto Index = linkCombinedIndex();
+ assert(Index);
// Save temps: index.
if (!SaveTempsDir.empty()) {
Index: llvm/lib/DWARFLinker/DWARFLinker.cpp
===================================================================
--- llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -2145,6 +2145,7 @@
Unit->markEverythingAsKept();
}
}
+ assert(Unit && "CompileUnit is not set!");
if (!Unit->getOrigUnit().getUnitDIE().hasChildren())
return Error::success();
if (!Quiet && Options.Verbose) {
Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===================================================================
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -2311,6 +2311,7 @@
bool HasFakeArg = false;
for (const auto *ArgRecord : ArgRecords) {
Args.emplace_back(createArgument(*ArgRecord, R.getName()));
+ assert(Args.back());
if (Header) {
Args.back()->writeDeclarations(OS);
OS << "\n\n";
@@ -2926,6 +2927,7 @@
Args.clear();
for (const auto *Arg : ArgRecords) {
Args.emplace_back(createArgument(*Arg, R.getName()));
+ assert(Args.back());
Args.back()->writePCHReadDecls(OS);
}
OS << " New = new (Context) " << R.getName() << "Attr(Context, Info";
@@ -2966,8 +2968,11 @@
OS << " Record.push_back(A->isImplicit());\n";
OS << " Record.push_back(A->isPackExpansion());\n";
- for (const auto *Arg : Args)
- createArgument(*Arg, R.getName())->writePCHWrite(OS);
+ for (const auto *Arg : Args) {
+ auto A = createArgument(*Arg, R.getName());
+ assert(A);
+ A->writePCHWrite(OS);
+ }
OS << " break;\n";
OS << " }\n";
}
@@ -3249,8 +3254,11 @@
<< " return false;\n";
std::vector<Record*> ArgRecords = R.getValueAsListOfDefs("Args");
- for (const auto *Arg : ArgRecords)
- createArgument(*Arg, R.getName())->writeASTVisitorTraversal(OS);
+ for (const auto *Arg : ArgRecords) {
+ auto A = createArgument(*Arg, R.getName());
+ assert(A);
+ A->writeASTVisitorTraversal(OS);
+ }
OS << " return true;\n";
OS << "}\n\n";
@@ -3313,8 +3321,10 @@
std::vector<std::unique_ptr<Argument>> Args;
Args.reserve(ArgRecords.size());
- for (const auto *ArgRecord : ArgRecords)
+ for (const auto *ArgRecord : ArgRecords) {
Args.emplace_back(createArgument(*ArgRecord, R.getName()));
+ assert(Args.back());
+ }
for (auto const &ai : Args)
ai->writeTemplateInstantiation(OS);
@@ -3368,7 +3378,9 @@
}
static bool isArgVariadic(const Record &R, StringRef AttrName) {
- return createArgument(R, AttrName)->isVariadic();
+ auto A = createArgument(R, AttrName);
+ assert(A);
+ return A->isVariadic();
}
static void emitArgInfo(const Record &R, raw_ostream &OS) {
@@ -3917,8 +3929,11 @@
SS << " OS << \" \" << A->getSpelling();\n";
Args = R.getValueAsListOfDefs("Args");
- for (const auto *Arg : Args)
- createArgument(*Arg, R.getName())->writeDump(SS);
+ for (const auto *Arg : Args) {
+ auto A = createArgument(*Arg, R.getName());
+ assert(A);
+ A->writeDump(SS);
+ }
if (SS.tell()) {
OS << " void Visit" << R.getName() << "Attr(const " << R.getName()
@@ -3945,8 +3960,11 @@
llvm::raw_string_ostream SS(FunctionContent);
Args = R.getValueAsListOfDefs("Args");
- for (const auto *Arg : Args)
- createArgument(*Arg, R.getName())->writeDumpChildren(SS);
+ for (const auto *Arg : Args) {
+ auto A = createArgument(*Arg, R.getName());
+ assert(A);
+ A->writeDumpChildren(SS);
+ }
if (SS.tell()) {
OS << " void Visit" << R.getName() << "Attr(const " << R.getName()
<< "Attr *A) {\n";
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
