[cfe-commits] r146633 - in /cfe/trunk: lib/Frontend/VerifyDiagnosticConsumer.cpp test/Analysis/taint-tester.c

Anna Zaks ganna at apple.com
Thu Dec 15 07:53:04 PST 2011


Ted,

I see how this more relaxed diagnostic option could be abused. So I do not mind pulling it out for that reason if someone feels strongly about it.

However, in this case, it's just the matter of convenience. For testing taint, I issue "taint" diagnostic on each tainted expression. In most cases, I do not care how many subexpressions are tainted on each line as long as taint gets propagated. The option to specify the exact number is still there and should be used when it's important.

Anna.

On Dec 14, 2011, at 9:27 PM, Ted Kremenek wrote:

> Hi Anna,
> 
> What is the particular motivation for this enhancement?  It seems to me that the output of the compiler should be deterministic, and our tests should test *exactly* the number of diagnostics issued.
> 
> Cheers,
> Ted
> 
> On Dec 14, 2011, at 6:28 PM, Anna Zaks wrote:
> 
>> Author: zaks
>> Date: Wed Dec 14 20:28:16 2011
>> New Revision: 146633
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=146633&view=rev
>> Log:
>> Add support for matching one or more (aka regex +) diagnostic messages with -verify.
>> 
>> Ex:
>> // expected-warning + {{tainted}
>> 
>> Modified:
>>   cfe/trunk/lib/Frontend/VerifyDiagnosticConsumer.cpp
>>   cfe/trunk/test/Analysis/taint-tester.c
>> 
>> Modified: cfe/trunk/lib/Frontend/VerifyDiagnosticConsumer.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/VerifyDiagnosticConsumer.cpp?rev=146633&r1=146632&r2=146633&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Frontend/VerifyDiagnosticConsumer.cpp (original)
>> +++ cfe/trunk/lib/Frontend/VerifyDiagnosticConsumer.cpp Wed Dec 14 20:28:16 2011
>> @@ -82,6 +82,9 @@
>>  static Directive* Create(bool RegexKind, const SourceLocation &Location,
>>                           const std::string &Text, unsigned Count);
>> public:
>> +  /// Constant representing one or more matches aka regex "+".
>> +  static const unsigned OneOrMoreCount =  UINT_MAX;
>> +
>>  SourceLocation Location;
>>  const std::string Text;
>>  unsigned Count;
>> @@ -276,10 +279,14 @@
>>    // skip optional whitespace
>>    PH.SkipWhitespace();
>> 
>> -    // next optional token: positive integer
>> +    // next optional token: positive integer or a '+'.
>>    unsigned Count = 1;
>>    if (PH.Next(Count))
>>      PH.Advance();
>> +    else if (PH.Next("+")) {
>> +      Count = Directive::OneOrMoreCount;
>> +      PH.Advance();
>> +    }
>> 
>>    // skip optional whitespace
>>    PH.SkipWhitespace();
>> @@ -420,6 +427,7 @@
>>  for (DirectiveList::iterator I = Left.begin(), E = Left.end(); I != E; ++I) {
>>    Directive& D = **I;
>>    unsigned LineNo1 = SourceMgr.getPresumedLineNumber(D.Location);
>> +    bool FoundOnce = false;
>> 
>>    for (unsigned i = 0; i < D.Count; ++i) {
>>      DiagList::iterator II, IE;
>> @@ -433,11 +441,16 @@
>>          break;
>>      }
>>      if (II == IE) {
>> +        if (D.Count == D.OneOrMoreCount && FoundOnce) {
>> +          // We are only interested in at least one match and we found one.
>> +          break;
>> +        }
>>        // Not found.
>>        LeftOnly.push_back(*I);
>>      } else {
>>        // Found. The same cannot be found twice.
>>        Right.erase(II);
>> +        FoundOnce = true;
>>      }
>>    }
>>  }
>> 
>> Modified: cfe/trunk/test/Analysis/taint-tester.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/taint-tester.c?rev=146633&r1=146632&r2=146633&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/Analysis/taint-tester.c (original)
>> +++ cfe/trunk/test/Analysis/taint-tester.c Wed Dec 14 20:28:16 2011
>> @@ -18,48 +18,48 @@
>>  int n;
>>  int *addr = &Buffer[0];
>>  scanf("%d", &n);
>> -  addr += n;// expected-warning 2 {{tainted}}
>> -  *addr = n; // expected-warning 3 {{tainted}}
>> +  addr += n;// expected-warning + {{tainted}}
>> +  *addr = n; // expected-warning + {{tainted}}
>> 
>> -  double tdiv = n / 30; // expected-warning 3 {{tainted}}
>> -  char *loc_cast = (char *) n; // expected-warning {{tainted}}
>> -  char tinc = tdiv++; // expected-warning {{tainted}}
>> -  int tincdec = (char)tinc--; // expected-warning 2 {{tainted}}
>> +  double tdiv = n / 30; // expected-warning+ {{tainted}}
>> +  char *loc_cast = (char *) n; // expected-warning +{{tainted}}
>> +  char tinc = tdiv++; // expected-warning + {{tainted}}
>> +  int tincdec = (char)tinc--; // expected-warning+{{tainted}}
>> 
>>  // Tainted ptr arithmetic/array element address.
>> -  int tprtarithmetic1 = *(addr+1); // expected-warning 2 {{tainted}}
>> +  int tprtarithmetic1 = *(addr+1); // expected-warning + {{tainted}}
>> 
>>  // Dereference.
>>  int *ptr;
>>  scanf("%p", &ptr);
>> -  int ptrDeref = *ptr; // expected-warning 2 {{tainted}}
>> -  int _ptrDeref = ptrDeref + 13; // expected-warning 2 {{tainted}}
>> +  int ptrDeref = *ptr; // expected-warning + {{tainted}}
>> +  int _ptrDeref = ptrDeref + 13; // expected-warning + {{tainted}}
>> 
>>  // Pointer arithmetic + dereferencing.
>>  // FIXME: We fail to propagate the taint here because RegionStore does not
>>  // handle ElementRegions with symbolic indexes.
>> -  int addrDeref = *addr; // expected-warning {{tainted}}
>> +  int addrDeref = *addr; // expected-warning + {{tainted}}
>>  int _addrDeref = addrDeref;
>> 
>>  // Tainted struct address, casts.
>>  struct XYStruct *xyPtr = 0;
>>  scanf("%p", &xyPtr);
>> -  void *tXYStructPtr = xyPtr; // expected-warning 2 {{tainted}}
>> -  struct XYStruct *xyPtrCopy = tXYStructPtr; // expected-warning 2 {{tainted}}
>> -  int ptrtx = xyPtr->x;// expected-warning 2 {{tainted}}
>> -  int ptrty = xyPtr->y;// expected-warning 2 {{tainted}}
>> +  void *tXYStructPtr = xyPtr; // expected-warning + {{tainted}}
>> +  struct XYStruct *xyPtrCopy = tXYStructPtr; // expected-warning + {{tainted}}
>> +  int ptrtx = xyPtr->x;// expected-warning + {{tainted}}
>> +  int ptrty = xyPtr->y;// expected-warning + {{tainted}}
>> 
>>  // Taint on fields of a struct.
>>  struct XYStruct xy = {2, 3, 11};
>>  scanf("%d", &xy.y);
>>  scanf("%d", &xy.x);
>> -  int tx = xy.x; // expected-warning {{tainted}}
>> +  int tx = xy.x; // expected-warning + {{tainted}}
>>  int ty = xy.y; // FIXME: This should be tainted as well.
>>  char ntz = xy.z;// no warning
>>  // Now, scanf scans both.
>>  scanf("%d %d", &xy.y, &xy.x);
>> -  int ttx = xy.x; // expected-warning {{tainted}}
>> -  int tty = xy.y; // expected-warning {{tainted}}
>> +  int ttx = xy.x; // expected-warning + {{tainted}}
>> +  int tty = xy.y; // expected-warning + {{tainted}}
>> }
>> 
>> void BitwiseOp(int in, char inn) {
>> @@ -67,22 +67,22 @@
>>  int m;
>>  int x = 0;
>>  scanf("%d", &x);
>> -  int y = (in << (x << in)) * 5;// expected-warning 4 {{tainted}}
>> +  int y = (in << (x << in)) * 5;// expected-warning + {{tainted}}
>>  // The next line tests integer to integer cast.
>> -  int z = y & inn; // expected-warning 2 {{tainted}}
>> -  if (y == 5) // expected-warning 2 {{tainted}}
>> -    m = z | z;// expected-warning 4 {{tainted}}
>> +  int z = y & inn; // expected-warning + {{tainted}}
>> +  if (y == 5) // expected-warning + {{tainted}}
>> +    m = z | z;// expected-warning + {{tainted}}
>>  else
>>    m = inn;
>> -  int mm = m; // expected-warning   {{tainted}}
>> +  int mm = m; // expected-warning + {{tainted}}
>> }
>> 
>> // Test getenv.
>> char *getenv(const char *name);
>> void getenvTest(char *home) {
>> -  home = getenv("HOME"); // expected-warning 2 {{tainted}}
>> -  if (home != 0) { // expected-warning 2 {{tainted}}
>> -      char d = home[0]; // expected-warning 2 {{tainted}}
>> +  home = getenv("HOME"); // expected-warning + {{tainted}}
>> +  if (home != 0) { // expected-warning + {{tainted}}
>> +      char d = home[0]; // expected-warning + {{tainted}}
>>    }
>> }
>> 
>> @@ -104,21 +104,21 @@
>>  fscanf(stdin, "%s %d", s, &t);
>>  // Note, here, s is not tainted, but the data s points to is tainted.
>>  char *ts = s;
>> -  char tss = s[0]; // expected-warning 1 {{tainted}}
>> -  int tt = t; // expected-warning 1 {{tainted}}
>> -  if((fp=fopen("test", "w")) == 0) // expected-warning 3 {{tainted}}
>> +  char tss = s[0]; // expected-warning + {{tainted}}
>> +  int tt = t; // expected-warning + {{tainted}}
>> +  if((fp=fopen("test", "w")) == 0) // expected-warning + {{tainted}}
>>    return 1;
>> -  fprintf(fp, "%s %d", s, t); // expected-warning 2 {{tainted}}
>> -  fclose(fp); // expected-warning 1 {{tainted}}
>> +  fprintf(fp, "%s %d", s, t); // expected-warning + {{tainted}}
>> +  fclose(fp); // expected-warning + {{tainted}}
>> 
>>  // Check if we propagate taint from stdin when it's used in an assignment.
>>  FILE *pfstd = stdin;
>>  fscanf(pfstd, "%s %d", s, &t); // TODO: This should be tainted as well.
>> 
>>  // Test fscanf and fopen.
>> -  if((fp=fopen("test","r")) == 0) // expected-warning 3 {{tainted}}
>> +  if((fp=fopen("test","r")) == 0) // expected-warning + {{tainted}}
>>    return 1;
>> -  fscanf(fp, "%s%d", s, &t); // expected-warning 1 {{tainted}}
>> -  fprintf(stdout, "%s %d", s, t); // expected-warning 1 {{tainted}}
>> +  fscanf(fp, "%s%d", s, &t); // expected-warning + {{tainted}}
>> +  fprintf(stdout, "%s %d", s, t); // expected-warning + {{tainted}}
>>  return 0;
>> }
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 




More information about the cfe-commits mailing list