Bug 2766

Summary: Remove logging from int64x64_t operator << to avoid stack overflow
Product: ns-3 Reporter: Alexander Krotov <krotov>
Component: coreAssignee: Peter Barnes <pdbarnes>
Status: RESOLVED FIXED    
Severity: normal CC: ns-bugs, tomh, tommaso.pecorella
Priority: P3    
Version: ns-3-dev   
Hardware: All   
OS: All   
Attachments: Proposed fix

Description Alexander Krotov 2017-07-13 05:12:53 UTC
Created attachment 2890 [details]
Proposed fix

Enabling all logging causes stack overflow because operator << of TimeWithUnit calls operator << of int64x64_t which uses NS_LOG_LOGIC. Logging calls operator << of TimeWithUnit again, so log line is never printed, stack overflow happens instead. I think it is better to just remove logging from int64x64_t operator <<.

There is a comment in int64x64.cc:

// Note:  Logging in this file is largely avoided due to the
// number of calls that are made to these functions and the possibility
// of causing recursions leading to stack overflow
Comment 1 Tommaso Pecorella 2017-07-13 12:20:12 UTC
To be honest there shouldn't be any LOG statement in a << operator, especially for low-level things like int64x64_t or similar.

Could you please check if there are more spare occurrences of NS_LOG in << operators and remove them ?

Thanks.
Comment 2 Alexander Krotov 2017-07-13 12:58:00 UTC
(In reply to Tommaso Pecorella from comment #1)
> To be honest there shouldn't be any LOG statement in a << operator,
> especially for low-level things like int64x64_t or similar.
> 
> Could you please check if there are more spare occurrences of NS_LOG in <<
> operators and remove them ?

The only thing I can find is that operator << for CommandLine (core/model/command-line.cc) calls PrintHelp, which has NS_LOG_FUNCTION.
Comment 3 Peter Barnes 2017-07-13 14:31:27 UTC
Just to be clear, the issue arises when enabling logging causes recursive calls to logging.

operator<< for CommandLine a) is required and b) has no logging itself.  CommandLine::PrintHelp just logs the function (NS_LOG_FUNCTION (this)), so it is not an issue.
Comment 4 Peter Barnes 2017-07-13 14:48:44 UTC
Again to be clear, I think the issue requires:

- logging enabled for the "int64x64" log component

- time prefix enabled on log messages (including prefix_all, which is the default)

I added the log statements to demonstrate that encoding/decoding of the Q64.64 format was being done correctly.  I'd prefer not to lose that functionality.

Alexander, could you please try this instead:

int64x64.cc:40:  NS_LOG_COMPONENT_DEFINE_MASK ("int64x64", LOG_PREFIX_TIME);

This will block printing of the time prefix on log messages from the int64x64 log component.
Comment 5 Alexander Krotov 2017-11-02 20:15:33 UTC
(In reply to Peter Barnes from comment #4)
> Again to be clear, I think the issue requires:
> 
> - logging enabled for the "int64x64" log component
> 
> - time prefix enabled on log messages (including prefix_all, which is the
> default)
> 
> I added the log statements to demonstrate that encoding/decoding of the
> Q64.64 format was being done correctly.  I'd prefer not to lose that
> functionality.
> 
> Alexander, could you please try this instead:
> 
> int64x64.cc:40:  NS_LOG_COMPONENT_DEFINE_MASK ("int64x64", LOG_PREFIX_TIME);
> 
> This will block printing of the time prefix on log messages from the
> int64x64 log component.

Tested it, ok with applying and marking as resolved.
Comment 6 Tom Henderson 2017-11-03 20:07:06 UTC
Pushed Peter's change in changeset 13135:5a046c9c4995