Extapi getenv handling

[mortkever]

Hello,

What is the intended handling of external functions like getenv(const char *name)? Currently get_env is annotated as "ALLOC_HEAP_RET" in the new method and as "EFT_NOSTRUCT_ALLOC" in the old method which seem inaccurate. This implies getenv returns a pointer to a newly allocated space on the heap/stack while getenv returns a pointer to the same memory when the argument is the same. So the same argument always returns the same pointer but different arguments return different pointers.

How is SVF supposed to handle functions like these? Would it be okay to annotate getenv as static and just consider all possible returns part of a larger struct, as that seems to be the actual workings of extern char ** environ?
Thanks!

[yuleisui]

This is a good observation. Yes, the same string as an argument of getenv should always return the same piece of memory. This requires string matching. We will need to add some implementation to support this. However, the input argument may not always be a constant, hence a string analysis (including data-flow of the arguments and all the string operation statements) is needed. The most conservative way is to use the static shared memory (global object).

[mortkever]

Great, I’ll look into making a pull request for the conservative approach.
I do see that there are more static return functions that don't have the static annotation (errno, localeconv, localtime, gmtime...). And that all signs of static returns were removed a few months ago. What is the current intended way to handle this?

[yuleisui]

Great, I’ll look into making a pull request for the conservative approach. I do see that there are more static return functions that don't have the static annotation (errno, localeconv, localtime, gmtime...). And that all signs of static returns were removed a few months ago. What is the current intended way to handle this?

@shuangxiangkan could you help with and reply this issue.

[adriaanjacobs]

It seems that SVF has gone through quite significant changes with respect to its handling of functions that return statically-allocated objects. Me and @mortkever have looked into the history here:

In the old JSON format, and even shortly after migrating to the new C-style format, SVF handled them via annotate("STATIC") (until SVF 2.7). This was perfectly sound & workable, am not aware of any reported issues here.

However, this was removed in #1188 in favor of a #define STATIC_OBJECT malloc(0) that got returned by functions that returned static objects. This approach introduced unsoundness (see #1610) because the STATIC_OBJECT expands to a new, heap-allocated object at every allocation site.

A subsequent PR removed the STATIC_OBJECT in favor of a new external function type, ALLOC_STACK_RET (#1616) which got uniformly applied to a bunch of functions, including many functions returning statically-allocated objects.

The aliasing behavior of this ALLOC_STACK_RET is unclear to me. Documentation says:

        ALLOC_STACK_RET,   // returns a pointer to a newly allocated stack object

but it is almost exclusively specified on functions that return statically-allocated objects? Like the ctype_* functions, asctime, the errnos, ... I would expect calls to "newly allocated stack objects" to not alias each other, while all calls that return statically-allocated objects should alias.

What is SVF's approach/intended behavior here? We're happy to submit some PRs to restore the previous behavior, but is there any high-level development direction w.r.t. the ExtAPI that we should be aware of?

[shuangxiangkan]

You're right that the current approach in SVF doesn't properly distinguish between functions returning statically allocated objects and those returning stack allocated objects. Using the same allocation method for both cases is indeed problematic. These two allocation types should indeed be handled differently.
I welcome you @adriaanjacobs and @mortkever , to submit new PRs to address this issue and I will further discuss these PRs with you.

[adriaanjacobs]

What was the original intention behind #1616 ? All functions marked "ALLOC_STACK_RET" were previously functions that returned STATIC_OBJECT. Should we just rename ALLOC_STACK_RET to STATIC_RET or so? Or does SVF want to specially handle stack objects somehow?

#1616 also contains other changes that seem to use the new ALLOC_STACK_RET annotations to infer that an object was allocated on the stack (which is almost always false)

Our current plan is to largely revert #1616 (keeping the naming changes ALLOC_RET->ALLOC_HEAP_RET)

[shuangxiangkan]

What was the original intention behind #1616 ? All functions marked "ALLOC_STACK_RET" were previously functions that returned STATIC_OBJECT. Should we just rename ALLOC_STACK_RET to STATIC_RET or so? Or does SVF want to specially handle stack objects somehow?

#1616 also contains other changes that seem to use the new ALLOC_STACK_RET annotations to infer that an object was allocated on the stack (which is almost always false)

Our current plan is to largely revert #1616 (keeping the naming changes ALLOC_RET->ALLOC_HEAP_RET)

The original design didn't distinguish between external functions allocating static objects or stack objects—it essentially treated them all as stack objects. So, simply reverting #1616 isn't enough, which is also why you didn't encounter build failures in #1688, as static object allocation for external functions wasn't addressed. To properly differentiate and handle static and stack objects allocations for external functions, some changes to how SVF processes external APIs might be needed.

[mortkever]

Our current train of thought is to remove the annotation for functions that return static objects and replace the function with this approach:

int errno_global;
__attribute__((alloc_size(4)))
int* __errno_location() {
    return &errno_global;
}

This way all calls to __errno_location return pointers to the same object. This also allows for different functions to return the same address as well. Gmtime and localtime for example, may return pointers to the same global object.
It also would require no specific handling in SVFIRExtApi.cpp as the functions behaviour would be described by the function body, the same as any other function with no annotation and only a body.

Would this work with the current handling of ExtApi functions? Specifically the addition of globals to the extapi.c file?