[cfe-dev] [clang-tidy] Check for paranoic code (pointer checking)

[Daniel Marjamäki via cfe-dev]

Hello!

I agree it’s controversial. I would say that some people would like that and others don’t. In my humble opinion it should be ok to add this check to clang-tidy.

Possibly to avoid some spurious warnings you could warn for const pointers only. I personally think that parameters that are written should be passed by pointer “getvalue(&x)” is more explicit imho than “getvalue(x)”.

> 3 - Are there more check ideas waiting for someone to implement so I could pick up, learn more and actually contribute to the code base?

I don’t know if there is an official list anywhere.

But I would assume there are many ideas.

Personally I would like that more types of UB is detected. Here is some testcode with various UB, Clang detects some of the UB but not all:


void alias() { int x; int *ip=&x; float *fp = (float *)ip; }



int buffer_overflow() { int x[10]={0}; return x[100]; }



int dead_pointer(int a) { int *p=&a; if (a) { int x=0; p = &x; } return *p; }



int division_by_zero() { return 100 / 0; }



int float_to_int() { double d=1E100; return (int)d; }



void negative_size(int sz) { if (sz < 0) { int buf[sz]; } }



int no_return() {}



int null_pointer() { int *p = 0; return *p; }



int *pointer_arithmetic() { static int buf[10]; return buf + 100; }



unsigned char pointer_to_u8() { static int buf[10]; return (int*)buf; }



int pointer_subtraction() { char a[10]; char b[10]; return b-a; }



int pointer_comparison() { char a[10]; char b[10]; return b<a; }



int shift_overrun(int x) { return x << 123; }



int shift_negative() { return -1 << 1; }



int int_overflow() { int intmax = (~0U) >> 1; return intmax * 2; }



void string_literal() { *((char *)"hello") = 0; }



int uninit() { int x; return x + 2; }



Best regards,
Daniel Marjamäki


From: cfe-dev [mailto:cfe-dev-bounces at lists.llvm.org] On Behalf Of Breno Guimarães via cfe-dev
Sent: Thursday, March 16, 2017 5:55 AM
To: cfe-dev at lists.llvm.org<mailto:cfe-dev at lists.llvm.org>
Subject: [cfe-dev] [clang-tidy] Check for paranoic code (pointer checking)

Hi all,

I started exploring clang-tidy to create checks for a large C++ code base we have at work, with many developers with C background doing C++.

After doing the first tutorial on VirtualShadowingCheck, I wrote a check to detect paranoic code using pointers. By paranoic, I mean this:

static void foo(T* p, ...) {
    if (!p) return;
    // do something
}

The method is doing the right thing to check the input, but the interface itself could've been written with a reference, thus delegating to the caller the responsability of doing the null check.

This is not at all always possible, since this style can be valid for several reasons: pointers can refer diverse conceptual objects, e.g. arrays), the function could be virtual, thus forced to follow the same parameters, and so on.

But still, controversial as it is, I ran this check on LLVM code base.

llvm/tools/clang/lib/Sema/SemaLookup.cpp:3453 is one example. (the check finds over 15 violations, but I haven't analyzed them all)

This function is one example that could be ignoring a potential mistake. The caller could expect a valid pointer in at least some of those calls. The convenience of writing:

foo(something->getPointer(), ...);

can stimulate the developer to not write checks/assertions for some cases that he/she expects the pointer to be valid.

All that being said, I have some questions:

1 - Is the community receptive to such controversial checks? Or is clang-tidy only interested in low false positive/ non controversial checks?
2 - Any feedback on this specific check?
3 - Are there more check ideas waiting for someone to implement so I could pick up, learn more and actually contribute to the code base?


Thanks,
Breno Rodrigues Guimarães



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20170316/be33f16a/attachment.html>