Invalid memory leak detection

[TomMD]

Using infer:

$ infer --version
Infer version v0.14.0-0bbdf63
Copyright 2009 - present Facebook. All Rights Reserved.

On libsodium we get an error:

src/libsodium/crypto_pwhash/argon2/argon2-core.c:493: error: MEMORY_LEAK
  memory dynamically allocated by call to `malloc()` at line 487, column 10 is not reachable after line 493, column 9.
  491.       result = allocate_memory(&(instance->region), instance->memory_blocks);
  492.       if (ARGON2_OK != result) {
  493. >         free_instance(instance, context->flags);
  494.           return result;
  495.       }

Summary of the reports

  MEMORY_LEAK: 1

But it appears my other machine with infer:

$ infer --version
Infer version v0.14.0-7c7b239
Copyright 2009 - present Facebook. All Rights Reserved.

Does not detect this "bug".

Notice the error is incorrect since the memory is freed by a function called prior to return. If we inline free_instances then the bug disappears.

[jvillard]

Thanks for the report. In the older version (7c7b239), is there a .specs file for free_instance in infer-out/specs/? (I think ls infer-out/specs/*free_instance*.specs should find it)

Will try and reproduce the issue locally.

[TomMD]

Yes there is a specs in the infer version 7c7b239:

$ infer --version
Infer version v0.14.0-7c7b239
Copyright 2009 - present Facebook. All Rights Reserved.
$ ls infer-out/specs/free_instance.b1df4c5116ec70ace90a14f717d1d23b.specs
infer-out/specs/free_instance.b1df4c5116ec70ace90a14f717d1d23b.specs
$ sha256sum infer-out/specs/free_instance.b1df4c5116ec70ace90a14f717d1d23b.specs
b71a6295d56633a9dfcfc7ebe83e2bd260ae75dfc8aa69f24cddf1618283f0f0  infer-out/specs/free_instance.b1df4c5116ec70ace90a14f717d1d23b.specs

I'm hosting the 3kb file while this ticket is open.

[TomMD]

Ah, I suppose what you might have actually wanted is the rendering, so that report is:

Procedure: free_instance
void free_instance(Argon2_instance_t* instance, int flags)
Analyzed
Phase: RE_EXECUTION
ERRORS:
WARNINGS:
FAILURE:NONE SYMOPS:187
PrePosts:
Quandary: { @val$0 -> (Footprint({ @val$0 }) ~> ?, { pseudo_rands -> { },
                                                     region -> { } }) }
Siof: ({ }, { })
RacerD:
Threads: NoThread, Locks: { }
Accesses { Thread: false Lock: false Pre: Owned({ 0 }) -> { Read of instance.memory_blocks at clear_memory at line 177, column 5,
                                                            Read of instance.pseudo_rands at clear_memory at line 177, column 5,
                                                            Read of instance.region at clear_memory at line 177, column 5,
                                                            Read of instance.segment_length at clear_memory at line 177, column 5,
                                                            Read of instance.region.base at free_memory at line 182, column 5,
                                                            Read of instance.region.size at free_memory at line 182, column 5,
                                                            Read of instance.pseudo_rands at  at line 180, column 5,
                                                            Write to instance.pseudo_rands at  at line 181, column 5,
                                                            Read of instance.region at  at line 182, column 5,
                                                            Write to instance.region at  at line 183, column 5 },
           Thread: false Lock: false Pre: False -> { Read of instance.region.memory at clear_memory at line 177, column 5 } }
Ownership: Unowned
Return Attributes: { }
Wobbly Paths: { instance.region, instance.pseudo_rands, n$8 }

[jvillard]

It looks like infer failed to find any specs for free_instance() in the analysis that detects memory leaks (see the PrePosts: empty entry), but it's not telling us why... I'll investigate, thanks!

[TomMD]

Argh, on this one I must apologize as well. It appears this is not a regression but simply a bug, perhaps related to #120? The slightly older infer does in fact have this false positive (observable here https://github.com/TomMD/libsodium/pull/14), my repository was likely not clean during my early test.

I'll edit the title of this issue to reflect the change.