New issue
Advanced search Search tips

Issue 580745 link

Starred by 2 users

Issue metadata

Status: Fixed
Closed: Jan 2016
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

issue 578344

Sign in to add a comment

HashMap::m_impl gets renamed wrong

Project Member Reported by, Jan 23 2016

Issue description

It becomes "impl" instead of "impl_". Why? Who knows.

Comment 1 by, Jan 23 2016

Blocking: chromium:578344

Comment 2 by, Jan 23 2016

Summary: HashMap::m_impl gets renamed wrong (was: HashMap::m_impl get's renamed wrong)

Comment 3 by, Jan 23 2016

Labels: -Type-Bug Type-Feature

Comment 4 by, Jan 27 2016

Status: Started

Comment 5 by, Jan 27 2016

This is something frightening going on.

If I do:

  const clang::RecordDecl* parent = decl.getParent();
  bool c = parent->isClass();
  if (c)
    name += '_';

Then it does the right thing.

If I do

  const clang::RecordDecl* parent = decl.getParent();
  if (parent->isClass())
    name += '_';

Then it does not. Tried building the tool with -fno-strict-aliasing to no effect.

Comment 6 by, Jan 27 2016

Status: Assigned
Um.. => assign to thakis maybe? I dunno where to go next with this.

FTR the change I was making is at

Comment 7 by, Jan 27 2016

Dana: what OS was this on?

Comment 8 by, Jan 27 2016

If you use the regular build setup, you build with gcc, not clang, right?

I do clang compiler bugs, but I don't do gcc compiler bugs :-P

I can verify if I can repro this though. (But not until I fixed my tooling crashes which are proving to be resilient :-( )

Comment 9 by, Jan 27 2016

@hans: goobuntu linux

I just run and ninja, so whatever it does. Maybe we could build this tool with the system clang?
No, we don't want to even hear about "system clang". There's only self-updating evergreen clang :-)

I think downloads a prebuilt gcc4.8 from somewhere. We could change it download something else.

It's pretty scary that this goes wrong. What did you change to build the tool with -fno-strict-aliasing?
My has these diffs

diff --git a/tools/clang/scripts/ b/tools/clang/scripts/
index c2fc850..1820e60 100755
--- a/tools/clang/scripts/
+++ b/tools/clang/scripts/
@@ -538,6 +538,8 @@ def UpdateClang(args):
     cflags += ['-DLLVM_FORCE_HEAD_REVISION']
     cxxflags += ['-DLLVM_FORCE_HEAD_REVISION']
+  cflags += ['-g -O0 -fno-strict-aliasing']
   deployment_env = None

I don't think -O0 is working, probably overridden by -O2 somewhere.
Huh, that adds those as a single flag, no? Does

  cflags += [ '-g', '-O0', '-fno-strict-aliasing' ]

help? (but i guess it's all joined together in the end, so probably not)
Yeh, I saw that too, it shouldn't. I'm trying with --bootstrap atm. dcheng suggested that might use clang to build the tool?
Well, with --bootstrap, prints/asserts seem to verify that it's doing the right thing. And worked correctly on HashMap.

gdb can't find the source file so breakpoints don't work tho. You win some you lose some.

So I guess one answer here is that we must use --bootstrap. Any pointers on making gdb would be super lovely tho.

=> me to update documentation.
Project Member

Comment 16 by, Jan 28 2016

The following revision refers to this bug:

commit 30d0f8c92a47b357789076692ec2eeb571bf312e
Author: danakj <>
Date: Thu Jan 28 00:26:33 2016

clang tools: Add --bootstrap to the documentation for

This lets us build the clang tools with clang not gcc, which appears to
have bugs!

Also work around the gcc bug in one place we know it happens anyways,
just because.

BUG= 580745 

Review URL:

Cr-Commit-Position: refs/heads/master@{#371933}


Status: Fixed
"Fixed" by using --bootstrap, though gdb stops functioning for the tool and chrome builds now take ~infinity time. D:
Yeah, goma can't work if you have a locally built clang of course :-) you can remove third_party/CR_clang_revision and rerun without args to get the remote-compatible clang binary again.

Not sure this should be marked fixed, we probably want to upload a GCC that doesn't randomly miscompile stuff. Our clang packages are all bootstrapped though, so they are fine.

Comment 19 by, Jan 28 2016

> Not sure this should be marked fixed, we probably want to upload a GCC that doesn't randomly miscompile stuff. Our clang packages are all bootstrapped though, so they are fine.


 bool c = parent->isClass();
  if (c)
    name += '_';


  if (parent->isClass())
    name += '_';

seems like a strange thing to miscompile though, and we do that kind of thing everywhere in Clang, which we've been building with that gcc for a long time.

It would be nice to have a better understanding of what's going on before we decide that a new GCC is the fix.
How does one debug further?

I did verify by changing the code and watching it branch or not that the parent->getTagKind() is not returning a valid TagTypeKind (i tried branching on <= TTK_Enum and that also didn't branch).

It clearly does branch for some (or most?) things in the blink codebase, but for some reason not for the HashMap::m_impl one.

Comment 21 by, Jan 28 2016

My comment was mostly aimed at thakis, as I don't think we should rush to build a new GCC without understand things better. I don't have any concrete ideas for debugging this more.

Sign in to add a comment