Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions clang/include/clang/3C/3C.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ struct _3COptions {
bool RemoveItypes;
bool ForceItypes;
#endif

// Currently applies only to the rewriting phase (because it is the only phase
// that generates diagnostics, except for the declaration merging diagnostics
// that are currently fatal) and uses the default "expected" prefix.
bool VerifyDiagnosticOutput;
};

// The main interface exposed by the 3C to interact with the tool.
Expand Down
71 changes: 52 additions & 19 deletions clang/lib/3C/3C.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ bool DisableCCTypeChecker;
bool WarnRootCause;
bool WarnAllRootCause;
std::set<std::string> FilePaths;
bool VerifyDiagnosticOutput;

#ifdef FIVE_C
bool RemoveItypes;
Expand Down Expand Up @@ -95,19 +96,39 @@ class RewriteAction : public ASTFrontendAction {

template <typename T>
std::unique_ptr<FrontendActionFactory>
newFrontendActionFactoryA(ProgramInfo &I) {
newFrontendActionFactoryA(ProgramInfo &I, bool VerifyTheseDiagnostics = false) {
class ArgFrontendActionFactory : public FrontendActionFactory {
public:
explicit ArgFrontendActionFactory(ProgramInfo &I) : Info(I) {}
explicit ArgFrontendActionFactory(ProgramInfo &I,
bool VerifyTheseDiagnostics)
: Info(I), VerifyTheseDiagnostics(VerifyTheseDiagnostics) {}

FrontendAction *create() override { return new T(Info); }

bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
FileManager *Files,
std::shared_ptr<PCHContainerOperations> PCHContainerOps,
DiagnosticConsumer *DiagConsumer) override {
if (VerifyTheseDiagnostics) {
// Mirroring the logic of clang::ParseDiagnosticArgs in
// clang/lib/Frontend/CompilerInvocation.cpp. In particular, note that
// VerifyPrefixes is assumed to be sorted, in case we add more in the
// future.
DiagnosticOptions &DiagOpts = Invocation->getDiagnosticOpts();
DiagOpts.VerifyDiagnostics = true;
DiagOpts.VerifyPrefixes.push_back("expected");
}
return FrontendActionFactory::runInvocation(
Invocation, Files, PCHContainerOps, DiagConsumer);
}

private:
ProgramInfo &Info;
bool VerifyTheseDiagnostics;
};

return std::unique_ptr<FrontendActionFactory>(
new ArgFrontendActionFactory(I));
new ArgFrontendActionFactory(I, VerifyTheseDiagnostics));
}

ArgumentsAdjuster getIgnoreCheckedPointerAdjuster() {
Expand Down Expand Up @@ -192,6 +213,7 @@ _3CInterface::_3CInterface(const struct _3COptions &CCopt,
AllocatorFunctions = CCopt.AllocatorFunctions;
WarnRootCause = CCopt.WarnRootCause || CCopt.WarnAllRootCause;
WarnAllRootCause = CCopt.WarnAllRootCause;
VerifyDiagnosticOutput = CCopt.VerifyDiagnosticOutput;

#ifdef FIVE_C
RemoveItypes = CCopt.RemoveItypes;
Expand Down Expand Up @@ -246,9 +268,11 @@ bool _3CInterface::buildInitialConstraints() {
std::unique_ptr<ToolAction> ConstraintTool = newFrontendActionFactoryA<
GenericAction<ConstraintBuilderConsumer, ProgramInfo>>(GlobalProgramInfo);

if (ConstraintTool)
Tool.run(ConstraintTool.get());
else
if (ConstraintTool) {
int ToolExitCode = Tool.run(ConstraintTool.get());
if (ToolExitCode != 0)
return false;
} else
llvm_unreachable("No action");

if (!GlobalProgramInfo.link()) {
Expand Down Expand Up @@ -297,9 +321,11 @@ bool _3CInterface::solveConstraints(bool ComputeInterimState) {
std::unique_ptr<ToolAction> ABInfTool = newFrontendActionFactoryA<
GenericAction<AllocBasedBoundsInference, ProgramInfo>>(
GlobalProgramInfo);
if (ABInfTool)
Tool.run(ABInfTool.get());
else
if (ABInfTool) {
int ToolExitCode = Tool.run(ABInfTool.get());
if (ToolExitCode != 0)
return false;
} else
llvm_unreachable("No Action");

// Propagate the information from allocator bounds.
Expand All @@ -310,9 +336,11 @@ bool _3CInterface::solveConstraints(bool ComputeInterimState) {
// after constraint solving but before rewriting.
std::unique_ptr<ToolAction> IMTool = newFrontendActionFactoryA<
GenericAction<IntermediateToolHook, ProgramInfo>>(GlobalProgramInfo);
if (IMTool)
Tool.run(IMTool.get());
else
if (IMTool) {
int ToolExitCode = Tool.run(IMTool.get());
if (ToolExitCode != 0)
return false;
} else
llvm_unreachable("No Action");

if (AllTypes) {
Expand Down Expand Up @@ -362,10 +390,13 @@ bool _3CInterface::writeConvertedFileToDisk(const std::string &FilePath) {
Tool.appendArgumentsAdjuster(getIgnoreCheckedPointerAdjuster());
std::unique_ptr<ToolAction> RewriteTool =
newFrontendActionFactoryA<RewriteAction<RewriteConsumer, ProgramInfo>>(
GlobalProgramInfo);
GlobalProgramInfo, VerifyDiagnosticOutput);

if (RewriteTool)
Tool.run(RewriteTool.get());
if (RewriteTool) {
int ToolExitCode = Tool.run(RewriteTool.get());
if (ToolExitCode != 0)
return false;
}
return true;
}
return false;
Expand All @@ -379,10 +410,12 @@ bool _3CInterface::writeAllConvertedFilesToDisk() {
// Rewrite the input files
std::unique_ptr<ToolAction> RewriteTool =
newFrontendActionFactoryA<RewriteAction<RewriteConsumer, ProgramInfo>>(
GlobalProgramInfo);
if (RewriteTool)
Tool.run(RewriteTool.get());
else
GlobalProgramInfo, VerifyDiagnosticOutput);
if (RewriteTool) {
int ToolExitCode = Tool.run(RewriteTool.get());
if (ToolExitCode != 0)
return false;
} else
llvm_unreachable("No action");

return true;
Expand Down
5 changes: 3 additions & 2 deletions clang/test/3C/partial_checked_arr.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
// RUN: 3c -addcr -alltypes %s | %clang -c -f3c-tool -fcheckedc-extension -x c -o %t1.unused -
// RUN: 3c -addcr %s | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s
// RUN: 3c -addcr %s | %clang -c -f3c-tool -fcheckedc-extension -x c -o %t2.unused -
// RUN: 3c -alltypes %s > %t
// RUN: 3c -alltypes %t | count 0
// RUN: 3c -alltypes -output-postfix=checked %s
// RUN: 3c -alltypes %S/partial_checked_arr.checked.c -- | count 0
// RUN: rm %S/partial_checked_arr.checked.c
Comment on lines -5 to +7
Copy link
Member

Choose a reason for hiding this comment

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

This is a regression to when we didn't know about %t temp files. Which admittedly may have been so soon that the changes overlapped. If there's no reason for the regressions, lets leave the simpler code that doesn't create extra files if it ends prematurely.

Copy link
Member Author

Choose a reason for hiding this comment

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

%t expanded to a filename ending in .c.tmp. For some reason, 3c doesn't like that as an input file name and spewed a bunch of errors. For now, I just changed this test to do the same thing as many of our other tests. When we have a proper solution to this problem, we can update all the tests.

I'm sorry for not giving you more time to review this.

Copy link
Member

Choose a reason for hiding this comment

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

It didn't give me any errors when I committed it. Is that because of tests on a different system, or related to the changes made in this pull request?

Copy link
Member Author

Choose a reason for hiding this comment

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

Before this pull request, when I ran the 3c command from the RUN line manually, I saw the errors, but 3c still exited 0, so the test passed, and lit doesn't seem to show the stderr of passing tests. With the changes in this pull request, 3c exited 1, so I had to do something about the test.

Copy link
Member

Choose a reason for hiding this comment

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

This seems significant to me, maybe for more issues? One for .tmp and one for passing with errors. If 3c ever allows tests to pass that have errors in them, that's bad for everyone. Did this pull request fix all of that? The OP was concerned with a subset and then you fixed more.

I guess for now we need to remember not to pass lit temp files to 3c. I'll see if there's a good place to fit that in the temp files issue I made earlier to document future changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

This seems significant to me, maybe for more issues? One for .tmp and one for passing with errors.

Agreed!

If 3c ever allows tests to pass that have errors in them, that's bad for everyone. Did this pull request fix all of that? The OP was concerned with a subset and then you fixed more.

I'm not sure, and I don't really want to spend more time researching it right now. Someone else is probably more familiar than I am with all the types of errors that can occur in 3c.

I guess for now we need to remember not to pass lit temp files to 3c. I'll see if there's a good place to fit that in the temp files issue I made earlier to document future changes.

Good. If we pass a temp file, the test will fail, and we just need to learn quickly that it is because of the temp file and not waste time debugging again.


int strcmp(const char *src1 : itype(_Nt_array_ptr<const char>),
const char *src2 : itype(_Nt_array_ptr<const char>));
Expand Down
36 changes: 12 additions & 24 deletions clang/test/3C/root_cause.c
Original file line number Diff line number Diff line change
@@ -1,55 +1,43 @@
// RUN: 3c -extra-arg="-Wno-everything" -alltypes -warn-root-cause %s 2>&1 >%t.unused | FileCheck %s
// RUN: 3c -extra-arg="-Wno-everything" -verify -alltypes -warn-root-cause %s

// This test is unusual in that it checks for the errors in the code

#include <stddef.h>
extern _Itype_for_any(T) void *malloc(size_t size) : itype(_Array_ptr<T>) byte_count(size);

void *x;
// CHECK-DAG: Default void* type
void *x; // expected-warning {{Default void* type}}

void test0() {
int *a;
char *b;
a = b;
// CHECK-DAG: Cast from int * to char *
a = b; // expected-warning {{Cast from char * to int *}}

int *c;
(char*) c;
// CHECK-DAG: Cast from int * to char *
(char*) c; // expected-warning {{Cast from int * to char *}}


int *e;
char *f;
f = (char*) e;
// CHECK-DAG: Cast from char * to int *
f = (char*) e; // expected-warning {{Cast from int * to char *}}
}

void test1() {
int a;
int *b;
b = malloc(sizeof(int));
b = malloc(sizeof(int)); // expected-warning {{Bad pointer type solution}}
b[0] = 1;

union u {
int *a;
// CHECK-DAG: Union or external struct field encountered
int *b;
// CHECK-DAG: Union or external struct field encountered
int *a; // expected-warning {{Union or external struct field encountered}}
int *b; // expected-warning {{Union or external struct field encountered}}
};

void (*c)(void);
c++;
// CHECK-DAG: Pointer arithmetic performed on a function pointer
c++; // expected-warning {{Pointer arithmetic performed on a function pointer}}

int *d = malloc(1);
// CHECK-DAG: Unsafe call to allocator function
int *d = malloc(1); // expected-warning {{Unsafe call to allocator function}}
}

extern int *glob;
// CHECK-DAG: External global variable glob has no definition
extern int *glob; // expected-warning {{External global variable glob has no definition}}

void (*f)(void *);
// CHECK-DAG: Default void* type

// CHECK-DAG: 11 warnings generated.
void (*f)(void *); // expected-warning {{Default void* type}}
15 changes: 15 additions & 0 deletions clang/tools/3c/3CStandalone.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,20 @@ static cl::opt<bool>
"even those unlikely to be interesting."),
cl::init(false), cl::cat(_3CCategory));

// https://clang.llvm.org/doxygen/classclang_1_1VerifyDiagnosticConsumer.html#details
//
// Analogous to the -verify option of `clang -cc1`, but currently applies only
// to the rewriting phase (because it is the only phase that generates
// diagnostics, except for the declaration merging diagnostics that are
// currently fatal). No checking of diagnostics from the other phases is
// performed. We cannot simply have the caller pass `-extra-arg=-Xclang
// -extra-arg=-verify` because that would expect each phase to produce the same
// set of diagnostics.
static cl::opt<bool> OptVerifyDiagnosticOutput(
"verify",
cl::desc("Verify diagnostic output (for automated testing of 3C)."),
cl::init(false), cl::cat(_3CCategory), cl::Hidden);

#ifdef FIVE_C
static cl::opt<bool> OptRemoveItypes(
"remove-itypes",
Expand Down Expand Up @@ -161,6 +175,7 @@ int main(int argc, const char **argv) {
CcOptions.DisableCCTypeChecker = OptDiableCCTypeChecker;
CcOptions.WarnRootCause = OptWarnRootCause;
CcOptions.WarnAllRootCause = OptWarnAllRootCause;
CcOptions.VerifyDiagnosticOutput = OptVerifyDiagnosticOutput;

#ifdef FIVE_C
CcOptions.RemoveItypes = OptRemoveItypes;
Expand Down