Hi, Jordan
As you say, this patch negatively effects other checker's behavior like missing malloc free. I'll try ways of SimpleStreamChecker and MallocChecker. Regards. Hiroo MATSUMOTO 2014-03-04 11:28 GMT+09:00 Jordan Rose <[email protected]>: > I don't think we want to assume the chroot always succeeds, and we > definitely don't want to stop processing if there's an error (because there > could be unrelated mistakes). Instead, we should check when we later access > the chroot-state that it hasn't already failed. > > That is, this test case should still warn: > > int *buf = malloc(4); > if (chroot("/") < 0) > return; // forgot to free buf! > > There's precedent for this in SimpleStreamChecker, which checks if a > symbol has been constrained to null before declaring that it's leaked, and > in MallocChecker, which tracks realloc failures to warn that the original > buffer is still live. Neither of those are quite what you'd want here, but > they're a good start. > > This checker isn't written very well anyway (for example, using FindGDM > and addGDM directly instead of going through ProgramState's get and set). > If you're interested, cleaning it up would be a nice improvement. > > Thanks for pointing out the errors. If you're not interested in solving > them yourself right now, please file a bug at http://llvm.org/bugs/. > > Jordan > > > On Mar 3, 2014, at 3:19 , Hiroo MATSUMOTO <[email protected]> wrote: > > > ChrootChecker tracks a chroot failed case. It will generate warning > > even though chroot is used properly. > > > > When finding improper using chroot, ChrootChecker doesn't stop > > tracking. It will generate verbose warning. > > > > For example, ChrootChecker will generate warnings from below code > > which can switch proper using and improper using with IMPROPER_USE. > > > > When IMPROPER_USE is not defined, 1 warning will be generated. > > When IMPROPER_USE is defined, 3 warnings will be generated. > > > > > > #include <stdio.h> > > #include <stdlib.h> > > #include <unistd.h> > > > > int main(int argc, char *argv[]) > > { > > if (argc < 2) { > > fprintf(stderr, "usage: %s newroot\n", argv[0]); > > return 1; > > } > > > > if (chroot(argv[1]) < 0) { > > perror("chroot"); /** proper using and improper using */ > > return 1; > > } > > > > #ifndef IMPROPER_USE > > if (chdir("/") < 0) { > > perror("chdir"); > > return 1; > > } > > #endif > > > > if (execv("/bin/sh", argv) < 0) { /** improper using */ > > perror("execv"); /** improper using */ > > return 1; > > } > > > > return 0; > > } > > > > > > This patch will bind return value of chroot to zero. And this patch > > will stop tracking when finding improper using chroot. > > > > > > Index: lib/StaticAnalyzer/Checkers/ChrootChecker.cpp > > =================================================================== > > --- lib/StaticAnalyzer/Checkers/ChrootChecker.cpp (revision 202679) > > +++ lib/StaticAnalyzer/Checkers/ChrootChecker.cpp (working copy) > > @@ -87,11 +87,13 @@ > > void ChrootChecker::Chroot(CheckerContext &C, const CallExpr *CE) const > { > > ProgramStateRef state = C.getState(); > > ProgramStateManager &Mgr = state->getStateManager(); > > + SValBuilder &svalBuilder = C.getSValBuilder(); > > + SVal success = > svalBuilder.makeZeroVal(svalBuilder.getContext().IntTy); > > > > // Once encouter a chroot(), set the enum value ROOT_CHANGED directly > in > > // the GDM. > > state = Mgr.addGDM(state, ChrootChecker::getTag(), (void*) > ROOT_CHANGED); > > - C.addTransition(state); > > + C.addTransition(state->BindExpr(CE, C.getLocationContext(), success)); > > } > > > > void ChrootChecker::Chdir(CheckerContext &C, const CallExpr *CE) const { > > @@ -140,7 +142,7 @@ > > void *const* k = C.getState()->FindGDM(ChrootChecker::getTag()); > > if (k) > > if (isRootChanged((intptr_t) *k)) > > - if (ExplodedNode *N = C.addTransition()) { > > + if (ExplodedNode *N = C.generateSink()) { > > if (!BT_BreakJail) > > BT_BreakJail.reset(new BuiltinBug( > > this, "Break out of jail", "No call of chdir(\"/\") > immediately " > > > > _______________________________________________ > > cfe-commits mailing list > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
