Remove the "ALLOC_STACK_RET" annotation from APIs that allocate static objects

[shuangxiangkan]

No description provided.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.61%. Comparing base (c63be58) to head (f29470e).
Report is 8 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1693      +/-   ##
==========================================
+ Coverage   63.58%   63.61%   +0.02%     
==========================================
  Files         244      244              
  Lines       25741    25758      +17     
  Branches     4510     4511       +1     
==========================================
+ Hits        16368    16385      +17     
  Misses       9373     9373              

Files with missing lines	Coverage Δ
svf-llvm/lib/LLVMModule.cpp	76.18% <100.00%> (+0.70%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[adriaanjacobs]

Thanks @shuangxiangkan, I think this is the right approach for statics. Two concerns:

1. The "ALLOC_STACK_RET" does not seem to be gone everywhere. In particular, the following functions still have it applied: __errno_location, getpwuid, localeconv, localtime, and setlocale. As far as I know, all of those return pointers to static storage as well? Based on the current list of functions in the ExtAPI, I'm not aware of any that would need the ALLOC_STACK_RET annotation, what do you think?
2. Every static function currently returns a pointer to a separate global variable. This is good as a starting point, but in reality some of these functions return pointers to the same underlying static objects. For instance, the asctime, ctime, gmtime and localtime functions can all return pointers to the same underlying static object. There may be others too, we would need to check the whole list

[shuangxiangkan]

Thanks for your feedback! This PR is to check if the current solution is reasonable, and I’ll keep optimizing it. Regarding your two concerns:

1. Some APIs still retain the "ALLOC_STACK_RET" annotation because removing them caused some test cases to fail. I’ll carefully review these APIs to confirm if they return pointers to static storage and fix the failing test cases.

2. I agree that some APIs may return the same underlying static objects. I’ll thoroughly check to identify which APIs share static objects. If you have time, I’d appreciate it if you could double-check to ensure all APIs returning static variables are handled correctly.

[shuangxiangkan]

Thanks @shuangxiangkan, I think this is the right approach for statics. Two concerns:

1. The "ALLOC_STACK_RET" does not seem to be gone everywhere. In particular, the following functions still have it applied: __errno_location, getpwuid, localeconv, localtime, and setlocale. As far as I know, all of those return pointers to static storage as well? Based on the current list of functions in the ExtAPI, I'm not aware of any that would need the ALLOC_STACK_RET annotation, what do you think?
2. Every static function currently returns a pointer to a separate global variable. This is good as a starting point, but in reality some of these functions return pointers to the same underlying static objects. For instance, the asctime, ctime, gmtime and localtime functions can all return pointers to the same underlying static object. There may be others too, we would need to check the whole list

Please take a look at the update — these two issues should be fixed now.

[adriaanjacobs]

Looks good to me! Are you also planning to merge SVF-tools/Test-Suite#157?

[yuleisui]

Looks good to me! Are you also planning to merge SVF-tools/Test-Suite#157?

Thanks for the confirmation. @shuangxiangkan can I merge it?

[yuleisui]

It has been fixed in the other pr here: #1696