Skip to content

Commit 5e303e8

Browse files
authored
[clang][analyzer] Add C standard streams to the internal memory space (#147766)
Variables `stdin`, `stdout`, `stderr` are now added to internal memory space instead of system memory space. The system memory space is invalidated at calls to system-defined functions which is not the correct behavior for the standard stream variables.
1 parent 9ef293e commit 5e303e8

File tree

2 files changed

+52
-6
lines changed

2 files changed

+52
-6
lines changed

clang/lib/StaticAnalyzer/Core/MemRegion.cpp

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,6 +1022,22 @@ getStackOrCaptureRegionForDeclContext(const LocationContext *LC,
10221022
return (const StackFrameContext *)nullptr;
10231023
}
10241024

1025+
static bool isStdStreamVar(const VarDecl *D) {
1026+
const IdentifierInfo *II = D->getIdentifier();
1027+
if (!II)
1028+
return false;
1029+
if (!D->getDeclContext()->isTranslationUnit())
1030+
return false;
1031+
StringRef N = II->getName();
1032+
QualType FILETy = D->getASTContext().getFILEType();
1033+
if (FILETy.isNull())
1034+
return false;
1035+
FILETy = FILETy.getCanonicalType();
1036+
QualType Ty = D->getType().getCanonicalType();
1037+
return Ty->isPointerType() && Ty->getPointeeType() == FILETy &&
1038+
(N == "stdin" || N == "stdout" || N == "stderr");
1039+
}
1040+
10251041
const VarRegion *MemRegionManager::getVarRegion(const VarDecl *D,
10261042
const LocationContext *LC) {
10271043
const auto *PVD = dyn_cast<ParmVarDecl>(D);
@@ -1054,10 +1070,18 @@ const VarRegion *MemRegionManager::getVarRegion(const VarDecl *D,
10541070
assert(!Ty.isNull());
10551071
if (Ty.isConstQualified()) {
10561072
sReg = getGlobalsRegion(MemRegion::GlobalImmutableSpaceRegionKind);
1057-
} else if (Ctx.getSourceManager().isInSystemHeader(D->getLocation())) {
1058-
sReg = getGlobalsRegion(MemRegion::GlobalSystemSpaceRegionKind);
10591073
} else {
1060-
sReg = getGlobalsRegion(MemRegion::GlobalInternalSpaceRegionKind);
1074+
// Pointer value of C standard streams is usually not modified by calls
1075+
// to functions declared in system headers. This means that they should
1076+
// not get invalidated by calls to functions declared in system headers,
1077+
// so they are placed in the global internal space, which is not
1078+
// invalidated by calls to functions declared in system headers.
1079+
if (Ctx.getSourceManager().isInSystemHeader(D->getLocation()) &&
1080+
!isStdStreamVar(D)) {
1081+
sReg = getGlobalsRegion(MemRegion::GlobalSystemSpaceRegionKind);
1082+
} else {
1083+
sReg = getGlobalsRegion(MemRegion::GlobalInternalSpaceRegionKind);
1084+
}
10611085
}
10621086

10631087
// Finally handle static locals.

clang/test/Analysis/stream.c

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -519,14 +519,36 @@ void reopen_std_stream(void) {
519519
if (!fp) return;
520520

521521
stdout = fp; // Let's make them alias.
522-
clang_analyzer_eval(fp == oldStdout); // expected-warning {{UNKNOWN}}
523-
clang_analyzer_eval(fp == stdout); // expected-warning {{TRUE}} no-FALSE
524-
clang_analyzer_eval(oldStdout == stdout); // expected-warning {{UNKNOWN}}
522+
clang_analyzer_eval(fp == oldStdout); // expected-warning {{FALSE}}
523+
clang_analyzer_eval(fp == stdout); // expected-warning {{TRUE}}
524+
clang_analyzer_eval(oldStdout == stdout); // expected-warning {{FALSE}}
525525
}
526526

527527
void only_success_path_does_not_alias_with_stdout(void) {
528528
if (stdout) return;
529529
FILE *f = fopen("/tmp/foof", "r"); // no-crash
530+
clang_analyzer_eval(f == 0);// expected-warning {{TRUE}} expected-warning {{FALSE}}
530531
if (!f) return;
531532
fclose(f);
532533
}
534+
535+
extern void do_something();
536+
537+
void test_no_invalidate_at_system_call() {
538+
FILE *old_stdin = stdin;
539+
if ((stdin = fopen("x/y/z", "r")) == NULL)
540+
return;
541+
clang_analyzer_eval(old_stdin == stdin); // expected-warning{{FALSE}}
542+
free(malloc(100));
543+
clang_analyzer_eval(old_stdin == stdin); // expected-warning{{FALSE}}
544+
fclose(stdin);
545+
}
546+
547+
void test_invalidate_at_non_system_call() {
548+
FILE *old_stdin = stdin;
549+
if ((stdin = fopen("x/y/z", "r")) == NULL)
550+
return;
551+
clang_analyzer_eval(old_stdin == stdin); // expected-warning{{FALSE}}
552+
do_something();
553+
clang_analyzer_eval(old_stdin == stdin); // expected-warning{{UNKNOWN}}
554+
}

0 commit comments

Comments
 (0)