New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 737847 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 696822



Sign in to add a comment

Transitively including flatbuffers.h from sources in some targets causes compile errors.

Project Member Reported by karandeepb@chromium.org, Jun 29 2017

Issue description

DefaultAllocator::instance() method in flatbuffers.h defines a static object,
leading to a destructor being run at exit-time. However, many targets in Chrome
use the "wexit_time_destructors" compiler config, which generates warnings when
exit time destructors are generated. This causes compile errors, when 
flatbuffers.h is transitively included from a file in one of such targets. 
 
Blocking: 696822
Cc: wvo@google.com
wvo@: Do you think a fix leaking the object would get accepted upstream. If yes, I can issue a patch. Else I'll just suppress the warning.
Cc: csharrison@chromium.org

Comment 4 by wvo@google.com, Jun 29 2017

Not sure what you mean by leak.. we certainly wouldn't want to dynamically allocate it I think.

The static allocator should not be initialized unless `instance` is called, so in theory an exit time destructor can be avoided by supplying your own instance of `DefaultAllocator`.. but this particular check is done statically, regardless of wether it is used?

Any other kind of fix (maybe an #ifdef ?) that is benign to other users will be accepted upstream.
Yeah I meant dynamically allocating it. I think it's a common practice to do so, at least in Chrome. See https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/FM8C-qPm5fk for reference. 

>> but this particular check is done statically, regardless of wether it is used?
The check uses the clang warning -Wexit-time-destructors, so it is done during compilation (assuming this is what you were asking).

Anyways, I'll try to suppress the warning for now, and see if the reviewers have any better ideas. Thanks.

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7fa8c45949cae56fa51370e432ae8d25627da0ba

commit 7fa8c45949cae56fa51370e432ae8d25627da0ba
Author: Karan Bhatia <karandeepb@chromium.org>
Date: Thu Jul 06 00:20:04 2017

Flatbuffers: Ignore exit time destructor warning.

DefaultAllocator::instance() method in flatbuffers.h defines a static object,
leading to a destructor being run at exit-time. However, many targets in Chrome
use the "wexit_time_destructors" compiler config, which generates warnings when
exit time destructors are generated. This causes compile errors, when 
flatbuffers.h is transitively included from a file in one of such targets. To
fix this, suppress the exit time destructor warning as part of the flatbuffer
config.

BUG= 737847 

Change-Id: I706be9e8c204ed24a82aa04b33b00a78cf36489d
Reviewed-on: https://chromium-review.googlesource.com/554318
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484403}
[modify] https://crrev.com/7fa8c45949cae56fa51370e432ae8d25627da0ba/third_party/flatbuffers/BUILD.gn

Status: Fixed (was: Assigned)

Sign in to add a comment