Opened 5 years ago

Closed 5 years ago

#13157 closed defect (fixed)

[patch] Clang Analyzer: Spurious Warnings 2

Reported by: teor Owned by:
Priority: Low Milestone: Tor: 0.2.6.x-final
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Keywords: tor-relay
Cc: nickm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

After #13036 was committed, clang --analyze continued to produce 2 existing NULL pointer dereference warnings; and 1 new uninitialised variable warning. I am compiling tor from git source on OS X.

01-in6-addr32-not-null.patch

The NULL pointer warnings on the return value of tor_addr_to_in6_addr32() are incorrect. But clang can't work this out itself due to limited analysis depth. To teach the analyser that the return value is safe to dereference, I applied tor_assert to the return value. This assert can optionally be wrapped in:

#if !defined(__clang_analyzer__) || PARANOIA

or similar, if performance is an issue in this code. The assertion silences the spurious warning.

02-dirserv-init-msg.patch

At this point in the code, msg has been set to a string constant. But the tor code checks that msg is not NULL, and the redundant NULL check confuses the analyser:

      log_info(LD_DIRSERV, "Router %s is now rejected: %s",
               description, msg?msg:"");

To avoid this spurious warning, the patch initialises msg to NULL.

clang --analyze clean!

Once these patches are applied, tor is clang --analyze clean, except for dead stores. (Which I trust the optimiser to remove in most cases.)

git version

These warnings occur in the git source of tor 0.2.6.?-alpha around 14 September 2014
e.g. commit d6b2a1709d28c656dadc019fb24145e6ac400771

Child Tickets

Attachments (2)

01-in6-addr32-not-null.patch (1.7 KB) - added by teor 5 years ago.
Teach clang --analyze that some return values are not NULL
02-dirserv-init-msg.patch (425 bytes) - added by teor 5 years ago.
Teach clang --analyze that a redundant NULL check doesn't suddenly make a variable garbage

Download all attachments as: .zip

Change History (3)

Changed 5 years ago by teor

Teach clang --analyze that some return values are not NULL

Changed 5 years ago by teor

Attachment: 02-dirserv-init-msg.patch added

Teach clang --analyze that a redundant NULL check doesn't suddenly make a variable garbage

comment:1 Changed 5 years ago by nickm

Milestone: Tor: 0.2.6.x-final
Resolution: fixed
Status: newclosed

Merged to master; thanks!

Note: See TracTickets for help on using tickets.