[help] scaffolding to start documenting issue types

Summary:
This provides some of the infrastructure needed for documentation issue
types within infer itself so we can generate the website and keep it up
to date when introducing new issue types.

Basically each issue type has documentation in its datatype in OCaml
now. But, documentation strings can be several pages of text! To avoid
making IssueType.ml even more unreadable, add the option to write
documentation in long form in files in infer/documentation/issues/.

This implements `infer help --help-issue-type` and show-cases how
documentation works for a couple of issue types. Next diff bulk-imports
the current website documentation in this form.

allow-large-files

Reviewed By: skcho

Differential Revision: D21934374

fbshipit-source-id: 2705bf424
master
Jules Villard 5 years ago committed by Facebook GitHub Bot
parent 1c0f242e8b
commit bc24cacd3a

@ -0,0 +1,97 @@
Infer reports null dereference bugs in C, Objective-C and Java. The issue is
about a pointer that can be `null` and it is dereferenced. This leads to a crash
in all the above languages.
### Null dereference in C
Here is an example of an inter-procedural null dereference bug in C:
```c
struct Person {
int age;
int height;
int weight;
};
int get_age(struct Person *who) {
return who->age;
}
int null_pointer_interproc() {
struct Person *joe = 0;
return get_age(joe);
}
```
### Null dereference in Objective-C
In Objective-C, null dereferences are less common than in Java, but they still
happen and their cause can be hidden. In general, passing a message to nil does
not cause a crash and returns `nil`, but dereferencing a pointer directly does
cause a crash as well as calling a `nil` block.C
```objectivec
-(void) foo:(void (^)())callback {
callback();
}
-(void) bar {
[self foo:nil]; //crash
}
```
Moreover, there are functions from the libraries that do not allow `nil` to be
passed as argument. Here are some examples:
```objectivec
-(void) foo {
NSString *str = nil;
NSArray *animals = @[@"horse", str, @"dolphin"]; //crash
}
-(void) bar {
CGColorSpaceRef colorSpace = CGColorSpaceCreateDeviceRGB(); //can return NULL
...
CFRelease(colorSpace); //crashes if called with NULL
}
```
### Null dereference in Java
Many of Infer's reports of potential NPE's come from code of the form
```java
p = foo(); // foo() might return null
stuff();
p.goo(); // dereferencing p, potential NPE
```
If you see code of this form, then you have several options.
<b> If you are unsure whether or not foo() will return null </b>, you should
ideally i. Change the code to ensure that foo() can not return null ii. Add a
check for whether p is null, and do something other than dereferencing p when it
is null.
Sometimes, in case ii it is not obvious what you should do when p is null. One
possibility (a last option) is to throw an exception, failing early. This can be
done using checkNotNull as in the following code:
```java
// code idiom for failing early
import static com.google.common.base.Preconditions.checkNotNull;
//... intervening code
p = checkNotNull(foo()); // foo() might return null
stuff();
p.goo(); // dereferencing p, potential NPE
```
The call checkNotNull(foo()) will never return null; in case foo() returns null
it fails early by throwing an NPE.
<b> If you are absolutely sure that foo() will not be null </b>, then if you
land your diff this case will no longer be reported after your diff makes it to
master. In the future we might include analysis directives (hey, analyzer, p is
not null!) like in Hack that tell the analyzer the information that you know,
but that is for later.

@ -0,0 +1,7 @@
This error is reported in C++. It fires when the initialization of a static
variable `A`, accesses a static variable `B` from another translation unit
(usually another `.cpp` file). There are no guarantees whether `B` has been
already initialized or not at that point.
For more technical definition and techniques to avoid/remediate, see the
[FAQ](https://isocpp.org/wiki/faq/ctors#static-init-order).

@ -3,6 +3,6 @@
; This source code is licensed under the MIT license found in the
; LICENSE file in the root directory of this source tree.
(dirs src bin)
(dirs documentation src bin)
(documentation (package infer) (mld_files :standard))

@ -35,6 +35,7 @@ module Unsafe : sig
{ unique_id: string
; checker: Checker.t
; visibility: visibility
; user_documentation: string option
; mutable default_severity: severity
; mutable enabled: bool
; mutable hum: string
@ -55,6 +56,7 @@ module Unsafe : sig
-> ?linters_def_file:string
-> id:string
-> ?visibility:visibility
-> ?user_documentation:string
-> severity
-> Checker.t
-> t
@ -75,6 +77,7 @@ end = struct
{ unique_id: string
; checker: Checker.t
; visibility: visibility
; user_documentation: string option
; mutable default_severity: severity
; mutable enabled: bool
; mutable hum: string
@ -119,12 +122,13 @@ end = struct
definitely. The [hum]an-readable description can be updated when we encounter the definition
of the issue type, eg in AL. *)
let register_from_string ?(enabled = true) ?hum:hum0 ?doc_url ?linters_def_file ~id:unique_id
?(visibility = User) default_severity checker =
?(visibility = User) ?user_documentation default_severity checker =
match find_from_string ~id:unique_id with
| ((Some
( { unique_id= _ (* we know it has to be the same *)
; checker= checker_old
; visibility= visibility_old
; user_documentation= _ (* new one must be [None] for dynamic issue types *)
; default_severity= _ (* mutable field to update *)
; enabled= _ (* not touching this one since [Config] will have set it *)
; hum= _ (* mutable field to update *)
@ -145,6 +149,12 @@ end = struct
die_of_mismatch ~what:"visibility"
~old:(string_of_visibility visibility_old)
~new_:(string_of_visibility visibility) ;
( match user_documentation with
| None ->
()
| Some user_documentation ->
L.die InternalError "Unexpected user documentation for issue type %s:@\n@\n%s@\n"
unique_id user_documentation ) ;
issue.default_severity <- default_severity ;
Option.iter hum0 ~f:(fun hum -> issue.hum <- hum) ;
if Option.is_some doc_url then issue.doc_url <- doc_url ;
@ -153,7 +163,15 @@ end = struct
| None ->
let hum = match hum0 with Some str -> str | _ -> prettify unique_id in
let issue =
{unique_id; visibility; default_severity; checker; enabled; hum; doc_url; linters_def_file}
{ unique_id
; visibility
; user_documentation
; default_severity
; checker
; enabled
; hum
; doc_url
; linters_def_file }
in
all_issues := IssueSet.add !all_issues issue ;
issue
@ -610,7 +628,10 @@ let mutable_local_variable_in_component_file =
register_from_string ~id:"MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE" Advice Linters
let null_dereference = register_from_string ~id:"NULL_DEREFERENCE" Error Biabduction
let null_dereference =
register_from_string ~id:"NULL_DEREFERENCE" Error Biabduction
~user_documentation:[%blob "../../documentation/issues/NULL_DEREFERENCE.md"]
let null_test_after_dereference =
register_from_string ~enabled:false ~id:"NULL_TEST_AFTER_DEREFERENCE" Warning Biabduction
@ -672,6 +693,7 @@ let starvation = register_from_string ~id:"STARVATION" ~hum:"UI Thread Starvatio
let static_initialization_order_fiasco =
register_from_string ~id:"STATIC_INITIALIZATION_ORDER_FIASCO" Error SIOF
~user_documentation:[%blob "../../documentation/issues/STATIC_INITIALIZATION_ORDER_FIASCO.md"]
let strict_mode_violation =

@ -25,6 +25,7 @@ type t = private
{ unique_id: string
; checker: Checker.t
; visibility: visibility
; user_documentation: string option
; mutable default_severity: severity
(** used for documentation but can be overriden at report time *)
; mutable enabled: bool
@ -51,6 +52,7 @@ val register_from_string :
-> ?linters_def_file:string
-> id:string
-> ?visibility:visibility
-> ?user_documentation:string
-> severity
-> Checker.t
-> t

@ -10,6 +10,10 @@
(libraries cmdliner core mtime.clock.os parmap re sqlite3 zip ATDGenerated
IStdlib OpenSource)
(preprocess
(pps ppx_compare ppx_enumerate)))
(pps ppx_blob ppx_compare ppx_enumerate))
(preprocessor_deps
(glob_files ../../documentation/checkers/*.md)
(glob_files ../../documentation/issues/*.md))
)
(documentation (package infer) (mld_files IBase))

@ -11,5 +11,5 @@
mtime.clock.os ocamlgraph oUnit parmap re sawja sledge sqlite3 str unix xmlm
yojson zarith zip CStubs)
(modules All_infer_in_one_file)
(preprocess (pps ppx_compare ppx_enumerate ppx_fields_conv ppx_hash ppx_sexp_conv ppx_variants_conv -no-check))
(preprocess (pps ppx_blob ppx_compare ppx_enumerate ppx_fields_conv ppx_hash ppx_sexp_conv ppx_variants_conv -no-check))
)

@ -23,6 +23,9 @@ let list_issue_types () =
~f:(fun ({ IssueType.unique_id
; checker
; visibility
; user_documentation=
_
(* do not show this as this can be a big multi-line string and not tool-friendly *)
; default_severity
; enabled
; hum
@ -40,6 +43,27 @@ let list_issue_types () =
let show_checkers _ = assert false
let show_issue_types _ = assert false
let show_issue_type (issue_type : IssueType.t) =
L.result "%s (unique ID: %s)@\n" issue_type.hum issue_type.unique_id ;
L.result "Reported by %s@\n" (Checker.get_id issue_type.checker) ;
match issue_type.user_documentation with
| None ->
L.result "No documentation@\n"
| Some documentation ->
L.result "Documentation:@\n @[" ;
let first_line = ref true in
String.split_lines documentation
|> List.iter ~f:(fun line ->
if not !first_line then L.result "@\n" ;
first_line := false ;
L.result "%s" line ) ;
L.result "@]@\n"
let show_issue_types issue_types =
L.result "@[" ;
List.iter ~f:show_issue_type issue_types ;
L.result "@]%!"
let write_website ~website_root:_ = assert false

@ -39,6 +39,7 @@ depends: [
"ocamlgraph"
"ounit" {>="2.0.5"}
"parmap" {>="1.0-rc8"}
"ppx_blob"
"ppx_compare" {>= "v0.14.0" & < "v0.15"}
"ppx_deriving" {>="4.1"}
"ppx_enumerate" {>= "v0.14.0" & < "v0.15"}

@ -87,6 +87,7 @@ depends: [
"ppx_base" {= "v0.14.0"}
"ppx_bench" {= "v0.14.0"}
"ppx_bin_prot" {= "v0.14.0"}
"ppx_blob" {= "0.7.0"}
"ppx_cold" {= "v0.14.0"}
"ppx_compare" {= "v0.14.0"}
"ppx_custom_printf" {= "v0.14.0"}

Loading…
Cancel
Save