diff --git a/sandboxed_api/tools/clang_generator/diagnostics.cc b/sandboxed_api/tools/clang_generator/diagnostics.cc index 25140e7..7d05aa5 100644 --- a/sandboxed_api/tools/clang_generator/diagnostics.cc +++ b/sandboxed_api/tools/clang_generator/diagnostics.cc @@ -14,7 +14,9 @@ #include "sandboxed_api/tools/clang_generator/diagnostics.h" +#include "absl/status/status.h" #include "absl/strings/cord.h" +#include "clang/Basic/Diagnostic.h" namespace sapi { @@ -22,8 +24,9 @@ constexpr absl::string_view kSapiStatusPayload = "https://github.com/google/sandboxed-api"; absl::Status MakeStatusWithDiagnostic(clang::SourceLocation loc, + absl::StatusCode code, absl::string_view message) { - absl::Status status = absl::UnknownError(message); + absl::Status status(code, message); absl::Cord payload; uint64_t raw_loc = loc.getRawEncoding(); payload.Append( @@ -32,6 +35,11 @@ absl::Status MakeStatusWithDiagnostic(clang::SourceLocation loc, return status; } +absl::Status MakeStatusWithDiagnostic(clang::SourceLocation loc, + absl::string_view message) { + return MakeStatusWithDiagnostic(loc, absl::StatusCode::kUnknown, message); +} + absl::optional GetDiagnosticLocationFromStatus( const absl::Status& status) { if (auto payload = @@ -43,14 +51,31 @@ absl::optional GetDiagnosticLocationFromStatus( return absl::nullopt; } -clang::DiagnosticBuilder ReportFatalError(clang::DiagnosticsEngine& de, - clang::SourceLocation loc, - absl::string_view message) { +namespace { + +clang::DiagnosticBuilder GetDiagnosticBuilder( + clang::DiagnosticsEngine& de, clang::SourceLocation loc, + clang::DiagnosticsEngine::Level level, absl::string_view message) { clang::DiagnosticBuilder builder = - de.Report(loc, de.getCustomDiagID(clang::DiagnosticsEngine::Fatal, - "header generation failed: %0")); + de.Report(loc, de.getCustomDiagID(level, "header generation: %0")); builder.AddString(llvm::StringRef(message.data(), message.size())); return builder; } +} // namespace + +clang::DiagnosticBuilder ReportFatalError(clang::DiagnosticsEngine& de, + clang::SourceLocation loc, + absl::string_view message) { + return GetDiagnosticBuilder(de, loc, clang::DiagnosticsEngine::Fatal, + message); +} + +clang::DiagnosticBuilder ReportWarning(clang::DiagnosticsEngine& de, + clang::SourceLocation loc, + absl::string_view message) { + return GetDiagnosticBuilder(de, loc, clang::DiagnosticsEngine::Warning, + message); +} + } // namespace sapi diff --git a/sandboxed_api/tools/clang_generator/diagnostics.h b/sandboxed_api/tools/clang_generator/diagnostics.h index 3727ed9..fbd0767 100644 --- a/sandboxed_api/tools/clang_generator/diagnostics.h +++ b/sandboxed_api/tools/clang_generator/diagnostics.h @@ -23,6 +23,12 @@ namespace sapi { +// Returns a new status with a payload that encodes the specified Clang source +// location. +absl::Status MakeStatusWithDiagnostic(clang::SourceLocation loc, + absl::StatusCode code, + absl::string_view message); + // Returns a new UNKNOWN status with a payload that encodes the specified Clang // source location. absl::Status MakeStatusWithDiagnostic(clang::SourceLocation loc, @@ -32,6 +38,10 @@ absl::Status MakeStatusWithDiagnostic(clang::SourceLocation loc, absl::optional GetDiagnosticLocationFromStatus( const absl::Status& status); +clang::DiagnosticBuilder ReportWarning(clang::DiagnosticsEngine& de, + clang::SourceLocation loc, + absl::string_view message); + clang::DiagnosticBuilder ReportFatalError(clang::DiagnosticsEngine& de, clang::SourceLocation loc, absl::string_view message); diff --git a/sandboxed_api/tools/clang_generator/emitter.cc b/sandboxed_api/tools/clang_generator/emitter.cc index aa5c842..9eeea4e 100644 --- a/sandboxed_api/tools/clang_generator/emitter.cc +++ b/sandboxed_api/tools/clang_generator/emitter.cc @@ -298,8 +298,9 @@ absl::StatusOr EmitFunction(const clang::FunctionDecl* decl) { auto function_name = ToStringView(decl->getName()); const clang::QualType return_type = decl->getDeclaredReturnType(); if (return_type->isRecordType()) { - return MakeStatusWithDiagnostic(decl->getBeginLoc(), - "returning record by value"); + return MakeStatusWithDiagnostic( + decl->getBeginLoc(), absl::StatusCode::kCancelled, + "returning record by value, skipping function"); } const bool returns_void = return_type->isVoidType(); @@ -320,9 +321,10 @@ absl::StatusOr EmitFunction(const clang::FunctionDecl* decl) { const clang::ParmVarDecl* param = decl->getParamDecl(i); if (param->getType()->isRecordType()) { return MakeStatusWithDiagnostic( - param->getBeginLoc(), + param->getBeginLoc(), absl::StatusCode::kCancelled, absl::StrCat("passing record parameter '", - ToStringView(param->getName()), "' by value")); + ToStringView(param->getName()), + "' by value, skipping function")); } ParameterInfo& param_info = params.emplace_back(); diff --git a/sandboxed_api/tools/clang_generator/emitter_test.cc b/sandboxed_api/tools/clang_generator/emitter_test.cc index 57b245f..fda55d5 100644 --- a/sandboxed_api/tools/clang_generator/emitter_test.cc +++ b/sandboxed_api/tools/clang_generator/emitter_test.cc @@ -165,34 +165,37 @@ TEST_F(EmitterTest, RemoveQualifiers) { ElementsAre("struct A { int data; }")); } -TEST_F(EmitterTest, StructByValueFails) { +TEST_F(EmitterTest, StructByValueSkipsFunction) { EmitterForTesting emitter; EXPECT_THAT( RunFrontendAction( R"(struct A { int data; }; extern "C" int Structize(A a);)", std::make_unique(emitter, GeneratorOptions())), - Not(IsOk())); + IsOk()); + EXPECT_THAT(emitter.functions_, IsEmpty()); } -TEST_F(EmitterTest, ReturnStructByValueFails) { +TEST_F(EmitterTest, ReturnStructByValueSkipsFunction) { EmitterForTesting emitter; EXPECT_THAT( RunFrontendAction( R"(struct A { int data; }; extern "C" A Structize();)", std::make_unique(emitter, GeneratorOptions())), - Not(IsOk())); + IsOk()); + EXPECT_THAT(emitter.functions_, IsEmpty()); } -TEST_F(EmitterTest, TypedefStructByValueFails) { +TEST_F(EmitterTest, TypedefStructByValueSkipsFunction) { EmitterForTesting emitter; EXPECT_THAT( RunFrontendAction( R"(typedef struct { int data; } A; extern "C" int Structize(A a);)", std::make_unique(emitter, GeneratorOptions())), - Not(IsOk())); + IsOk()); + EXPECT_THAT(emitter.functions_, IsEmpty()); } TEST(IncludeGuard, CreatesRandomizedGuardForEmptyFilename) { diff --git a/sandboxed_api/tools/clang_generator/generator.cc b/sandboxed_api/tools/clang_generator/generator.cc index 65887d9..8095653 100644 --- a/sandboxed_api/tools/clang_generator/generator.cc +++ b/sandboxed_api/tools/clang_generator/generator.cc @@ -78,6 +78,7 @@ void GeneratorASTConsumer::HandleTranslationUnit(clang::ASTContext& context) { ReportFatalError(context.getDiagnostics(), context.getTranslationUnitDecl()->getBeginLoc(), "AST traversal exited early"); + return; } emitter_.AddTypesFiltered(visitor_.collector_.collected()); @@ -85,10 +86,13 @@ void GeneratorASTConsumer::HandleTranslationUnit(clang::ASTContext& context) { for (clang::FunctionDecl* func : visitor_.functions_) { absl::Status status = emitter_.AddFunction(func); if (!status.ok()) { - ReportFatalError( - context.getDiagnostics(), - GetDiagnosticLocationFromStatus(status).value_or(func->getBeginLoc()), - status.message()); + clang::SourceLocation loc = + GetDiagnosticLocationFromStatus(status).value_or(func->getBeginLoc()); + if (absl::IsCancelled(status)) { + ReportWarning(context.getDiagnostics(), loc, status.message()); + continue; + } + ReportFatalError(context.getDiagnostics(), loc, status.message()); break; } }