Chromium C++ help needed

[Tom Callaway]

Hi folks,

I spent some time this weekend trying to get Chromium 72 building on
Fedora, but I kept running into a C++ issue that I was not able to resolve.
This happened with gcc-9.0.1-0.8.fc30.x86_64 and gcc-8.3.1-2.fc29.x86_64.

Here's a sample of the error (it happens in a few places), from Fedora 30:

In file included from
../../base/trace_event/trace_event_system_stats_monitor.h:15,
                 from ../../base/trace_event/trace_event.h:26,
                 from ../../base/threading/scoped_blocking_call.cc:11:
../../base/trace_event/trace_log.h: In constructor
'base::ScopedBlockingCall::ScopedBlockingCall(base::BlockingType)':
../../base/threading/scoped_blocking_call.cc:88:5:   in 'constexpr'
expansion of
'base::trace_event::TraceLog::GetBuiltinCategoryEnabled(((const
char*)"base"))'
../../base/trace_event/trace_log.h:215:25: error: '((&
base::trace_event::CategoryRegistry::categories_[7]) != 0)' is not a
constant expression
  215 |     if (builtin_category)
         |                                 ^

Now, chromium isn't the easiest code base to work with, but what seems to
be happening is that this code calls one of the TRACE_EVENT macros, like
this from base/threading/scoped_blocking_call.cc:

TRACE_EVENT_BEGIN1("base", "ScopedBlockingCall",
"blocking_type",
static_cast<int>(blocking_type));

Those macros are defined in base/trace_event/common/trace_event_common.h:

#define TRACE_EVENT_BEGIN1(category_group, name, arg1_name, arg1_val)     \
  INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_BEGIN, category_group, name, \
                           TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val)

INTERNAL_TRACE_EVENT_ADD() is defined in base/trace_event/trace_event.h:

/ Implementation detail: internal macro to create static category and add
// event if the category is enabled.
#define INTERNAL_TRACE_EVENT_ADD(phase, category_group, name, flags, ...)  \
  do {                                                                     \
    INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category_group);                \
    if (INTERNAL_TRACE_EVENT_CATEGORY_GROUP_ENABLED()) {                   \
      trace_event_internal::AddTraceEvent(                                 \
          phase, INTERNAL_TRACE_EVENT_UID(category_group_enabled), name,   \
          trace_event_internal::kGlobalScope, trace_event_internal::kNoId, \
          flags, trace_event_internal::kNoId, ##__VA_ARGS__);              \
    }                                                                      \
  } while (0)

INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO is also defined in
base/trace_event/trace_event.h:

#define INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category_group)
   \
  static_assert(
   \

base::trace_event::BuiltinCategories::IsAllowedCategory(category_group), \
      "Unknown tracing category is used. Please register your "
    \
      "category in base/trace_event/builtin_categories.h");
    \
  constexpr const unsigned char* INTERNAL_TRACE_EVENT_UID(
   \
      k_category_group_enabled) =
    \

base::trace_event::TraceLog::GetBuiltinCategoryEnabled(category_group);  \
  const unsigned char* INTERNAL_TRACE_EVENT_UID(category_group_enabled);
   \
  INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO_MAYBE_AT_COMPILE_TIME(
    \
      category_group, INTERNAL_TRACE_EVENT_UID(k_category_group_enabled),
    \
      INTERNAL_TRACE_EVENT_UID(category_group_enabled));

Finally, inside here, it
calls base::trace_event::TraceLog::GetBuiltinCategoryEnabled(), which is
defined in base/trace_event/trace_log.h:

  // Called by TRACE_EVENT* macros, don't call this directly.
  // The name parameter is a category group for example:
  // TRACE_EVENT0("renderer,webkit", "WebViewImpl::HandleInputEvent")
  static const unsigned char* GetCategoryGroupEnabled(const char* name);
  static const char* GetCategoryGroupName(
      const unsigned char* category_group_enabled);
  static constexpr const unsigned char* GetBuiltinCategoryEnabled(
      const char* name) {
    TraceCategory* builtin_category =
        CategoryRegistry::GetBuiltinCategoryByName(name);
    if (builtin_category)
      return builtin_category->state_ptr();
    return nullptr;
  }

Okay, so what seems like is happening here, is that the calls like this:

TRACE_EVENT_BEGIN1("base", "ScopedBlockingCall",
"blocking_type",
static_cast<int>(blocking_type));

Are passing "base" (that first var) all the way into
GetCategoryGroupEnabled, which is finding it via GetBuiltinCategoryByName()
in base/trace_event/category_registry.h, checking against the list in
INTERNAL_TRACE_LIST_BUILTIN_CATEGORIES(X)
from base/trace_event/builtin_categories.h, finding it, then returning this
as builtin_category:

(& base::trace_event::CategoryRegistry::categories_[7])

When if(builtin_category) is run (trying to check to see if we got a
buillt-in category or a nullptr, I think), thats when GCC says:

error: '((& base::trace_event::CategoryRegistry::categories_[7]) != 0)' is
not a constant expression

*****

None of the other distros that build Chromium seem to have hit this issue,
nor does it appear in any bugs I could find with upstream (but they use a
fork of clang to build, and we use gcc). I have hit the limit of my C++
knowledge here, so I'm not sure how to fix this. If you can help me, it
would be greatly appreciated. My work in progress is checked into the
master chromium branch in git.

There are quite a few security issues that this new version fixes,
including some that are being actively exploited, so timely help here is
appreciated.

Thanks,

~tom

[Vascom]

Look at chromium-vaapi build in rpmfusion.

пн, 11 мар. 2019 г., 20:17 Tom Callaway <tcallawa(a)redhat.com&gt;:

>  Hi folks,
>
>  I spent some time this weekend trying to get Chromium 72 building on
>  Fedora, but I kept running into a C++ issue that I was not able to resolve.
>  This happened with gcc-9.0.1-0.8.fc30.x86_64 and gcc-8.3.1-2.fc29.x86_64.
>
>  Here's a sample of the error (it happens in a few places), from Fedora 30:
>
>  In file included from
>  ../../base/trace_event/trace_event_system_stats_monitor.h:15,
>                   from ../../base/trace_event/trace_event.h:26,
>                   from ../../base/threading/scoped_blocking_call.cc:11:
>  ../../base/trace_event/trace_log.h: In constructor
>  'base::ScopedBlockingCall::ScopedBlockingCall(base::BlockingType)':
>  ../../base/threading/scoped_blocking_call.cc:88:5:   in 'constexpr'
>  expansion of
>  'base::trace_event::TraceLog::GetBuiltinCategoryEnabled(((const
>  char*)"base"))'
>  ../../base/trace_event/trace_log.h:215:25: error: '((&
>  base::trace_event::CategoryRegistry::categories_[7]) != 0)' is not a
>  constant expression
>    215 |     if (builtin_category)
>           |                                 ^
>
>  Now, chromium isn't the easiest code base to work with, but what seems to
>  be happening is that this code calls one of the TRACE_EVENT macros, like
>  this from base/threading/scoped_blocking_call.cc:
>
>  TRACE_EVENT_BEGIN1("base", "ScopedBlockingCall",
> "blocking_type",
>  static_cast<int>(blocking_type));
>
>  Those macros are defined in base/trace_event/common/trace_event_common.h:
>
>  #define TRACE_EVENT_BEGIN1(category_group, name, arg1_name, arg1_val)     \
>    INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_BEGIN, category_group, name, \
>                             TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val)
>
>  INTERNAL_TRACE_EVENT_ADD() is defined in base/trace_event/trace_event.h:
>
>  / Implementation detail: internal macro to create static category and add
>  // event if the category is enabled.
>  #define INTERNAL_TRACE_EVENT_ADD(phase, category_group, name, flags, ...)
>  \
>    do {
>   \
>      INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category_group);
>  \
>      if (INTERNAL_TRACE_EVENT_CATEGORY_GROUP_ENABLED()) {
>   \
>        trace_event_internal::AddTraceEvent(
>   \
>            phase, INTERNAL_TRACE_EVENT_UID(category_group_enabled), name,
>   \
>            trace_event_internal::kGlobalScope, trace_event_internal::kNoId,
>  \
>            flags, trace_event_internal::kNoId, ##__VA_ARGS__);
>  \
>      }
>  \
>    } while (0)
>
>  INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO is also defined in
>  base/trace_event/trace_event.h:
>
>  #define INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category_group)
>       \
>    static_assert(
>       \
>
>  base::trace_event::BuiltinCategories::IsAllowedCategory(category_group), \
>        "Unknown tracing category is used. Please register your "
>      \
>        "category in base/trace_event/builtin_categories.h");
>      \
>    constexpr const unsigned char* INTERNAL_TRACE_EVENT_UID(
>       \
>        k_category_group_enabled) =
>      \
>
>  base::trace_event::TraceLog::GetBuiltinCategoryEnabled(category_group);  \
>    const unsigned char* INTERNAL_TRACE_EVENT_UID(category_group_enabled);
>       \
>    INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO_MAYBE_AT_COMPILE_TIME(
>      \
>        category_group, INTERNAL_TRACE_EVENT_UID(k_category_group_enabled),
>      \
>        INTERNAL_TRACE_EVENT_UID(category_group_enabled));
>
>  Finally, inside here, it
>  calls base::trace_event::TraceLog::GetBuiltinCategoryEnabled(), which is
>  defined in base/trace_event/trace_log.h:
>
>    // Called by TRACE_EVENT* macros, don't call this directly.
>    // The name parameter is a category group for example:
>    // TRACE_EVENT0("renderer,webkit",
> "WebViewImpl::HandleInputEvent")
>    static const unsigned char* GetCategoryGroupEnabled(const char* name);
>    static const char* GetCategoryGroupName(
>        const unsigned char* category_group_enabled);
>    static constexpr const unsigned char* GetBuiltinCategoryEnabled(
>        const char* name) {
>      TraceCategory* builtin_category =
>          CategoryRegistry::GetBuiltinCategoryByName(name);
>      if (builtin_category)
>        return builtin_category->state_ptr();
>      return nullptr;
>    }
>
>  Okay, so what seems like is happening here, is that the calls like this:
>
>  TRACE_EVENT_BEGIN1("base", "ScopedBlockingCall",
> "blocking_type",
>  static_cast<int>(blocking_type));
>
>  Are passing "base" (that first var) all the way into
>  GetCategoryGroupEnabled, which is finding it via GetBuiltinCategoryByName()
>  in base/trace_event/category_registry.h, checking against the list in
>  INTERNAL_TRACE_LIST_BUILTIN_CATEGORIES(X)
>  from base/trace_event/builtin_categories.h, finding it, then returning this
>  as builtin_category:
>
>  (& base::trace_event::CategoryRegistry::categories_[7])
>
>  When if(builtin_category) is run (trying to check to see if we got a
>  buillt-in category or a nullptr, I think), thats when GCC says:
>
>  error: '((& base::trace_event::CategoryRegistry::categories_[7]) != 0)' is
>  not a constant expression
>
>  *****
>
>  None of the other distros that build Chromium seem to have hit this issue,
>  nor does it appear in any bugs I could find with upstream (but they use a
>  fork of clang to build, and we use gcc). I have hit the limit of my C++
>  knowledge here, so I'm not sure how to fix this. If you can help me, it
>  would be greatly appreciated. My work in progress is checked into the
>  master chromium branch in git.
>
>  There are quite a few security issues that this new version fixes,
>  including some that are being actively exploited, so timely help here is
>  appreciated.
>
>  Thanks,
>
>  ~tom
>
>  _______________________________________________
>  devel mailing list -- devel(a)lists.fedoraproject.org
>  To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
>  Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>  List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives:
>  https://lists.fedoraproject.org/archives/list/[e-mail]
>

[Tom Callaway]

FWIW, I did. There is no fix there.

~tom

On Mon, Mar 11, 2019 at 1:20 PM Vascom <vascom2(a)gmail.com&gt; wrote:

>  Look at chromium-vaapi build in rpmfusion.
>
>  пн, 11 мар. 2019 г., 20:17 Tom Callaway <tcallawa(a)redhat.com&gt;:
>
> > Hi folks,
> >
> > I spent some time this weekend trying to get Chromium 72 building on
> > Fedora, but I kept running into a C++ issue that I was not able to resolve.
> > This happened with gcc-9.0.1-0.8.fc30.x86_64 and gcc-8.3.1-2.fc29.x86_64.
> >
> > Here's a sample of the error (it happens in a few places), from Fedora 30:
> >
> > In file included from
> > ../../base/trace_event/trace_event_system_stats_monitor.h:15,
> >                  from ../../base/trace_event/trace_event.h:26,
> >                  from ../../base/threading/scoped_blocking_call.cc:11:
> > ../../base/trace_event/trace_log.h: In constructor
> > 'base::ScopedBlockingCall::ScopedBlockingCall(base::BlockingType)':
> > ../../base/threading/scoped_blocking_call.cc:88:5:   in 'constexpr'
> > expansion of
> > 'base::trace_event::TraceLog::GetBuiltinCategoryEnabled(((const
> > char*)"base"))'
> > ../../base/trace_event/trace_log.h:215:25: error: '((&
> > base::trace_event::CategoryRegistry::categories_[7]) != 0)' is not a
> > constant expression
> >   215 |     if (builtin_category)
> >          |                                 ^
> >
> > Now, chromium isn't the easiest code base to work with, but what seems to
> > be happening is that this code calls one of the TRACE_EVENT macros, like
> > this from base/threading/scoped_blocking_call.cc:
> >
> > TRACE_EVENT_BEGIN1("base", "ScopedBlockingCall",
> "blocking_type",
> > static_cast<int>(blocking_type));
> >
> > Those macros are defined in base/trace_event/common/trace_event_common.h:
> >
> > #define TRACE_EVENT_BEGIN1(category_group, name, arg1_name, arg1_val)
> >  \
> >   INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_BEGIN, category_group, name,
> > \
> >                            TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val)
> >
> > INTERNAL_TRACE_EVENT_ADD() is defined in base/trace_event/trace_event.h:
> >
> > / Implementation detail: internal macro to create static category and add
> > // event if the category is enabled.
> > #define INTERNAL_TRACE_EVENT_ADD(phase, category_group, name, flags,
> > ...)  \
> >   do {
> >  \
> >     INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category_group);
> >   \
> >     if (INTERNAL_TRACE_EVENT_CATEGORY_GROUP_ENABLED()) {
> >  \
> >       trace_event_internal::AddTraceEvent(
> >  \
> >           phase, INTERNAL_TRACE_EVENT_UID(category_group_enabled), name,
> >  \
> >           trace_event_internal::kGlobalScope,
> > trace_event_internal::kNoId, \
> >           flags, trace_event_internal::kNoId, ##__VA_ARGS__);
> >   \
> >     }
> >   \
> >   } while (0)
> >
> > INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO is also defined in
> > base/trace_event/trace_event.h:
> >
> > #define INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category_group)
> >      \
> >   static_assert(
> >      \
> >
> > base::trace_event::BuiltinCategories::IsAllowedCategory(category_group), \
> >       "Unknown tracing category is used. Please register your "
> >       \
> >       "category in base/trace_event/builtin_categories.h");
> >       \
> >   constexpr const unsigned char* INTERNAL_TRACE_EVENT_UID(
> >      \
> >       k_category_group_enabled) =
> >       \
> >
> > base::trace_event::TraceLog::GetBuiltinCategoryEnabled(category_group);  \
> >   const unsigned char* INTERNAL_TRACE_EVENT_UID(category_group_enabled);
> >      \
> >   INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO_MAYBE_AT_COMPILE_TIME(
> >       \
> >       category_group,
> > INTERNAL_TRACE_EVENT_UID(k_category_group_enabled),      \
> >       INTERNAL_TRACE_EVENT_UID(category_group_enabled));
> >
> > Finally, inside here, it
> > calls base::trace_event::TraceLog::GetBuiltinCategoryEnabled(), which is
> > defined in base/trace_event/trace_log.h:
> >
> >   // Called by TRACE_EVENT* macros, don't call this directly.
> >   // The name parameter is a category group for example:
> >   // TRACE_EVENT0("renderer,webkit",
> "WebViewImpl::HandleInputEvent")
> >   static const unsigned char* GetCategoryGroupEnabled(const char* name);
> >   static const char* GetCategoryGroupName(
> >       const unsigned char* category_group_enabled);
> >   static constexpr const unsigned char* GetBuiltinCategoryEnabled(
> >       const char* name) {
> >     TraceCategory* builtin_category =
> >         CategoryRegistry::GetBuiltinCategoryByName(name);
> >     if (builtin_category)
> >       return builtin_category->state_ptr();
> >     return nullptr;
> >   }
> >
> > Okay, so what seems like is happening here, is that the calls like this:
> >
> > TRACE_EVENT_BEGIN1("base", "ScopedBlockingCall",
> "blocking_type",
> > static_cast<int>(blocking_type));
> >
> > Are passing "base" (that first var) all the way into
> > GetCategoryGroupEnabled, which is finding it via GetBuiltinCategoryByName()
> > in base/trace_event/category_registry.h, checking against the list in
> > INTERNAL_TRACE_LIST_BUILTIN_CATEGORIES(X)
> > from base/trace_event/builtin_categories.h, finding it, then returning this
> > as builtin_category:
> >
> > (& base::trace_event::CategoryRegistry::categories_[7])
> >
> > When if(builtin_category) is run (trying to check to see if we got a
> > buillt-in category or a nullptr, I think), thats when GCC says:
> >
> > error: '((& base::trace_event::CategoryRegistry::categories_[7]) != 0)'
> > is not a constant expression
> >
> > *****
> >
> > None of the other distros that build Chromium seem to have hit this
> > issue, nor does it appear in any bugs I could find with upstream (but they
> > use a fork of clang to build, and we use gcc). I have hit the limit of my
> > C++ knowledge here, so I'm not sure how to fix this. If you can help me, it
> > would be greatly appreciated. My work in progress is checked into the
> > master chromium branch in git.
> >
> > There are quite a few security issues that this new version fixes,
> > including some that are being actively exploited, so timely help here is
> > appreciated.
> >
> > Thanks,
> >
> > ~tom
> >
> > _______________________________________________
> > devel mailing list -- devel(a)lists.fedoraproject.org
> > To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
> > Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
> > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> > List Archives:
> > https://lists.fedoraproject.org/archives/list/[e-mail]
> >
>  _______________________________________________
>  devel mailing list -- devel(a)lists.fedoraproject.org
>  To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
>  Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>  List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives:
>  https://lists.fedoraproject.org/archives/list/[e-mail]
>

[Jakub Jelinek]

On Mon, Mar 11, 2019 at 01:16:07PM -0400, Tom Callaway wrote:
>  I spent some time this weekend trying to get Chromium 72 building on
>  Fedora, but I kept running into a C++ issue that I was not able to resolve.
>  This happened with gcc-9.0.1-0.8.fc30.x86_64 and gcc-8.3.1-2.fc29.x86_64. 
Can you please provide preprocessed source + g++ command line options,
from the snippets it is hard to see what's going on.
From the description it seems maybe like:
template <int N>
struct S {
  static constexpr int a[2] = { 1, 2 };
};
static_assert (&S<0>::a[1] != nullptr);

which g++ accepts for -std=c++{11,14} but rejects for -std=c++{17,2a} when
S<0>::a is an inline variable.  I think we have a similar
http://gcc.gnu.org/PR89074 .  The middle-end punts here and doesn't optimize
the != NULL to true because it is address of a comdat variable and thus it
in the end could come up from any other TU.  Though perhaps in these cases
the standard gives us some guarantees.

	Jakub

[Tom Callaway]

Here's a g++ invocation (Fedora 30):

g++ -MMD -MF obj/base/base/memory_pressure_listener.o.d -DUSE_SYMBOLIZE
-DV8_DEPRECATION_WARNINGS -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1
-DUSE_NSS_CERTS=1 -DUSE_X11=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD
-DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D__STDC_CONSTANT_MACROS
-D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -DCOMPONENT_BUILD -DNDEBUG
-DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DBASE_IMPLEMENTATION
-DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_32
-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_26 -I../.. -Igen
-fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector
-Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__=
-funwind-tables -fPIC -pipe -pthread -m64 -march=x86-64 -Wall
-Wno-unused-local-typedefs -Wno-maybe-uninitialized
-Wno-deprecated-declarations -Wno-comments -Wno-missing-field-initializers
-Wno-unused-parameter -fno-omit-frame-pointer -g0 -fvisibility=hidden
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -O2 -fno-ident
-fdata-sections -ffunction-sections -std=gnu++14 -Wno-narrowing
-fno-exceptions -fno-rtti -fvisibility-inlines-hidden
-fno-delete-null-pointer-checks -c
../../base/memory/memory_pressure_listener.cc -o
obj/base/base/memory_pressure_listener.o

Preprocessed source is attached.

On Mon, Mar 11, 2019 at 1:31 PM Jakub Jelinek <jakub(a)redhat.com&gt; wrote:

>  On Mon, Mar 11, 2019 at 01:16:07PM -0400, Tom Callaway wrote:
>  > I spent some time this weekend trying to get Chromium 72 building on
>  > Fedora, but I kept running into a C++ issue that I was not able to
>  resolve.
>  > This happened with gcc-9.0.1-0.8.fc30.x86_64 and gcc-8.3.1-2.fc29.x86_64.
>
>  Can you please provide preprocessed source + g++ command line options,
>  from the snippets it is hard to see what's going on.
>  From the description it seems maybe like:
>  template <int N>
>  struct S {
>    static constexpr int a[2] = { 1, 2 };
>  };
>  static_assert (&S<0>::a[1] != nullptr);
>
>  which g++ accepts for -std=c++{11,14} but rejects for -std=c++{17,2a} when
>  S<0>::a is an inline variable.  I think we have a similar
>  http://gcc.gnu.org/PR89074 .  The middle-end punts here and doesn't
>  optimize
>  the != NULL to true because it is address of a comdat variable and thus it
>  in the end could come up from any other TU.  Though perhaps in these cases
>  the standard gives us some guarantees.
>
>          Jakub
>

[Tom Callaway]

I tried removing some of the compiler flags to see if I could identify what
might be triggering this, and removing "-fno-delete-null-pointer-checks"
seems to make this error vanish.

Not sure if that helps or not, but hopefully, I can get this beast building
without it.

Thanks,
Tom

On Mon, Mar 11, 2019 at 1:31 PM Jakub Jelinek <jakub(a)redhat.com&gt; wrote:

>  On Mon, Mar 11, 2019 at 01:16:07PM -0400, Tom Callaway wrote:
>  > I spent some time this weekend trying to get Chromium 72 building on
>  > Fedora, but I kept running into a C++ issue that I was not able to
>  resolve.
>  > This happened with gcc-9.0.1-0.8.fc30.x86_64 and gcc-8.3.1-2.fc29.x86_64.
>
>  Can you please provide preprocessed source + g++ command line options,
>  from the snippets it is hard to see what's going on.
>  From the description it seems maybe like:
>  template <int N>
>  struct S {
>    static constexpr int a[2] = { 1, 2 };
>  };
>  static_assert (&S<0>::a[1] != nullptr);
>
>  which g++ accepts for -std=c++{11,14} but rejects for -std=c++{17,2a} when
>  S<0>::a is an inline variable.  I think we have a similar
>  http://gcc.gnu.org/PR89074 .  The middle-end punts here and doesn't
>  optimize
>  the != NULL to true because it is address of a comdat variable and thus it
>  in the end could come up from any other TU.  Though perhaps in these cases
>  the standard gives us some guarantees.
>
>          Jakub
>

[Jakub Jelinek]

On Wed, Mar 13, 2019 at 10:28:29AM -0400, Tom Callaway wrote:
>  I tried removing some of the compiler flags to see if I could
> identify what
>  might be triggering this, and removing "-fno-delete-null-pointer-checks"
>  seems to make this error vanish. 
-fno-delete-null-pointer-checks is certainly an option that requests that
the compiler does not fold comparisons against NULL at compile time based on
assumption that objects have non-NULL address, so sure, if it has such asserts,
it is incompatible with that option, you can't have both.

	Jakub

[Tom Callaway]

The odd thing is that this compiler flag is hardcoded into the build by the
upstream. I'm wondering why Fedora hits this when no one else seems to.

I mean, I'm fine to disable it, because the Chromium codebase is a tomb of
horrors anyway, and if I have to sacrifice that flag to make it happy, so
be it.

~tom

On Wed, Mar 13, 2019 at 10:37 AM Jakub Jelinek <jakub(a)redhat.com&gt; wrote:

>  On Wed, Mar 13, 2019 at 10:28:29AM -0400, Tom Callaway wrote:
>  > I tried removing some of the compiler flags to see if I could identify
>  what
>  > might be triggering this, and removing "-fno-delete-null-pointer-checks"
>  > seems to make this error vanish.
>
>  -fno-delete-null-pointer-checks is certainly an option that requests that
>  the compiler does not fold comparisons against NULL at compile time based
>  on
>  assumption that objects have non-NULL address, so sure, if it has such
>  asserts,
>  it is incompatible with that option, you can't have both.
>
>          Jakub
>

[Jakub Jelinek]

On Wed, Mar 13, 2019 at 12:49:35PM -0400, Tom Callaway wrote:
>  The odd thing is that this compiler flag is hardcoded into the build
> by the
>  upstream. I'm wondering why Fedora hits this when no one else seems to.
>  
>  I mean, I'm fine to disable it, because the Chromium codebase is a tomb of
>  horrors anyway, and if I have to sacrifice that flag to make it happy, so
>  be it. 
Most likely the flag has been added to workaround other bugs in the
codebase, but maybe those were already fixed and flag forgotten.
The flag can be used to workaround bugs like calling memcpy/memset etc.
with NULL argument (when the size is 0), or calling methods on NULL pointers
to objects etc.

	Jakub

On Wed, Mar 13, 2019 at 11:53 AM, Jakub Jelinek <jakub(a)redhat.com&gt; 
wrote:
>  calling methods on NULL pointers to objects etc. 
Chromium probably does this (or, at least, did in the past). Removing 
-fno-delete-null-pointer-checks could introduce Fedora-specific crashes.

Michael

[Tom Hughes]

On 13/03/2019 17:03, mcatanzaro(a)gnome.org wrote:
>  On Wed, Mar 13, 2019 at 11:53 AM, Jakub Jelinek
> <jakub(a)redhat.com&gt; wrote:
> > calling methods on NULL pointers to objects etc.
>  
>  Chromium probably does this (or, at least, did in the past). Removing 
>  -fno-delete-null-pointer-checks could introduce Fedora-specific crashes. 
Yes v8 (and hence chromium) were well known for triggering crashes
when gcc first started assuming that people wouldn't call methods
on null pointers.

The underlying bugs in v8 where it was doing that may well be fixed
by now of course.

Tom

-- 
Tom Hughes (tom(a)compton.nu)
http://compton.nu/

[Christophe de Dinechin]

>  On 11 Mar 2019, at 18:16, Tom Callaway <tcallawa(a)redhat.com&gt;
> wrote:
>  
>  Hi folks,
>  
>  I spent some time this weekend trying to get Chromium 72 building on Fedora, but I kept
> running into a C++ issue that I was not able to resolve. This happened with
> gcc-9.0.1-0.8.fc30.x86_64 and gcc-8.3.1-2.fc29.x86_64. 
>  
>  Here's a sample of the error (it happens in a few places), from Fedora 30:
>  
>  In file included from ../../base/trace_event/trace_event_system_stats_monitor.h:15,
>                  from ../../base/trace_event/trace_event.h:26,
>                  from ../../base/threading/scoped_blocking_call.cc:11:
>  ../../base/trace_event/trace_log.h: In constructor
> 'base::ScopedBlockingCall::ScopedBlockingCall(base::BlockingType)':
>  ../../base/threading/scoped_blocking_call.cc:88:5:   in 'constexpr' expansion of
> 'base::trace_event::TraceLog::GetBuiltinCategoryEnabled(((const
> char*)"base"))'
>  ../../base/trace_event/trace_log.h:215:25: error: '((&
> base::trace_event::CategoryRegistry::categories_[7]) != 0)' is not a constant
> expression 
It looks like the compiler is not smart enough to detect that this is the address of a
static object, defined at line 92744 of your preprocessed file as:

 static TraceCategory categories_[kMaxCategories];

whereas it is smart enough to go through GetBuiltnCategoryByName and figure out index 7
from the name…

I tried to compile your preprocessed fragment with both clang or gcc. Interestingly:

1) Without your command-line options, I don’t see the same error.

2) With your command-line options, I see the error with gcc, but not with clang.

So I suspect a compiler bug.



>   215 |     if (builtin_category)
>          |                                 ^
>  
>  Now, chromium isn't the easiest code base to work with, but what seems to be
> happening is that this code calls one of the TRACE_EVENT macros, like this from
> base/threading/scoped_blocking_call.cc:
>  
>  TRACE_EVENT_BEGIN1("base", "ScopedBlockingCall",
> "blocking_type", static_cast<int>(blocking_type));
>  
>  Those macros are defined in base/trace_event/common/trace_event_common.h:
>  
>  #define TRACE_EVENT_BEGIN1(category_group, name, arg1_name, arg1_val)     \
>   INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_BEGIN, category_group, name, \
>                            TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val)
>  
>  INTERNAL_TRACE_EVENT_ADD() is defined in base/trace_event/trace_event.h:
>  
>  / Implementation detail: internal macro to create static category and add
>  // event if the category is enabled.
>  #define INTERNAL_TRACE_EVENT_ADD(phase, category_group, name, flags, ...)  \
>   do {                                                                     \
>     INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category_group);                \
>     if (INTERNAL_TRACE_EVENT_CATEGORY_GROUP_ENABLED()) {                   \
>       trace_event_internal::AddTraceEvent(                                 \
>           phase, INTERNAL_TRACE_EVENT_UID(category_group_enabled), name,   \
>           trace_event_internal::kGlobalScope, trace_event_internal::kNoId, \
>           flags, trace_event_internal::kNoId, ##__VA_ARGS__);              \
>     }                                                                      \
>   } while (0)
>  
>  INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO is also defined in
> base/trace_event/trace_event.h:
>  
>  #define INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category_group)                 \
>   static_assert(                                                               \
>       base::trace_event::BuiltinCategories::IsAllowedCategory(category_group), \
>       "Unknown tracing category is used. Please register your "               
> \
>       "category in base/trace_event/builtin_categories.h");                   
> \
>   constexpr const unsigned char* INTERNAL_TRACE_EVENT_UID(                     \
>       k_category_group_enabled) =                                              \
>       base::trace_event::TraceLog::GetBuiltinCategoryEnabled(category_group);  \
>   const unsigned char* INTERNAL_TRACE_EVENT_UID(category_group_enabled);       \
>   INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO_MAYBE_AT_COMPILE_TIME(                \
>       category_group, INTERNAL_TRACE_EVENT_UID(k_category_group_enabled),      \
>       INTERNAL_TRACE_EVENT_UID(category_group_enabled));
>  
>  Finally, inside here, it calls base::trace_event::TraceLog::GetBuiltinCategoryEnabled(),
> which is defined in base/trace_event/trace_log.h:
>  
>   // Called by TRACE_EVENT* macros, don't call this directly.
>   // The name parameter is a category group for example:
>   // TRACE_EVENT0("renderer,webkit", "WebViewImpl::HandleInputEvent")
>   static const unsigned char* GetCategoryGroupEnabled(const char* name);
>   static const char* GetCategoryGroupName(
>       const unsigned char* category_group_enabled);
>   static constexpr const unsigned char* GetBuiltinCategoryEnabled(
>       const char* name) {
>     TraceCategory* builtin_category =
>         CategoryRegistry::GetBuiltinCategoryByName(name);
>     if (builtin_category)
>       return builtin_category->state_ptr();
>     return nullptr;
>   }
>  
>  Okay, so what seems like is happening here, is that the calls like this:
>  
>  TRACE_EVENT_BEGIN1("base", "ScopedBlockingCall",
> "blocking_type", static_cast<int>(blocking_type));
>  
>  Are passing "base" (that first var) all the way into GetCategoryGroupEnabled,
> which is finding it via GetBuiltinCategoryByName() in
> base/trace_event/category_registry.h, checking against the list in
> INTERNAL_TRACE_LIST_BUILTIN_CATEGORIES(X) from base/trace_event/builtin_categories.h,
> finding it, then returning this as builtin_category: 
>  
>  (& base::trace_event::CategoryRegistry::categories_[7])
>  
>  When if(builtin_category) is run (trying to check to see if we got a buillt-in category
> or a nullptr, I think), thats when GCC says:
>  
>  error: '((& base::trace_event::CategoryRegistry::categories_[7]) != 0)' is
> not a constant expression
>  
>  *****
>  
>  None of the other distros that build Chromium seem to have hit this issue, nor does it
> appear in any bugs I could find with upstream (but they use a fork of clang to build, and
> we use gcc). I have hit the limit of my C++ knowledge here, so I'm not sure how to fix
> this. If you can help me, it would be greatly appreciated. My work in progress is checked
> into the master chromium branch in git.
>  
>  There are quite a few security issues that this new version fixes, including some that
> are being actively exploited, so timely help here is appreciated.
>  
>  Thanks,
>  
>  ~tom
>  
>  _______________________________________________
>  devel mailing list -- devel(a)lists.fedoraproject.org
>  To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
>  Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>  List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives:
> https://lists.fedoraproject.org/archives/list/[e-mail]

[Gary Gatling]

On Tue, Mar 12, 2019 at 7:57 AM Christophe de Dinechin <dinechin(a)redhat.com&gt;
wrote:

>  I tried to compile your preprocessed fragment with both clang or gcc.
>  Interestingly:
>
>  1) Without your command-line options, I don’t see the same error.
>
>  2) With your command-line options, I see the error with gcc, but not with
>  clang.
>
>  So I suspect a compiler bug.
>  
Could chromium be built with clang in fedora? Or does it have to be built
with gcc?