FAQ
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
cgo: process DWARF info even when debug data is used for value

Always process the DWARF info, even when the const value is determined
using the debug data block. This ensures that the injected enum is
removed and future loads of the same constant do not trigger
inconsistent definitions.

Fixes issue 4054.

Please review this at http://codereview.appspot.com/6501101/

Affected files:
M src/cmd/cgo/gcc.go


Index: src/cmd/cgo/gcc.go
===================================================================
--- a/src/cmd/cgo/gcc.go
+++ b/src/cmd/cgo/gcc.go
@@ -616,10 +616,7 @@
n.FuncType = conv.FuncType(f, pos)
} else {
n.Type = conv.Type(types[i], pos)
- // Prefer debug data over DWARF debug output, if we have it.
- if n.Kind == "const" && i < len(enumVal) {
- n.Const = fmt.Sprintf("%#x", enumVal[i])
- } else if enums[i] != 0 && n.Type.EnumValues != nil {
+ if enums[i] != 0 && n.Type.EnumValues != nil {
k := fmt.Sprintf("__cgo_enum__%d", i)
n.Kind = "const"
n.Const = fmt.Sprintf("%#x", n.Type.EnumValues[k])
@@ -627,6 +624,10 @@
// equally in future loads of the same constant.
delete(n.Type.EnumValues, k)
}
+ // Prefer debug data over DWARF debug output, if we have it.
+ if n.Kind == "const" && i < len(enumVal) {
+ n.Const = fmt.Sprintf("%#x", enumVal[i])
+ }
}
}

Search Discussions

  • Fullung at Sep 8, 2012 at 5:55 am
    Hello

    Maybe a test?

    There's been other stuff like this that also didn't spawn tests:

    http://code.google.com/p/go/source/detail?r=aeaab9df5600

    but a problem I reported with a union did:

    http://code.google.com/p/go/source/detail?r=c3b10fce42

    so that might give some ideas for where to put the test.

    It seems enums keep breaking, so they could really use a test.

    http://code.google.com/p/go/issues/detail?id=207
    http://code.google.com/p/go/issues/detail?id=479
    http://code.google.com/p/go/issues/detail?id=4054

    Regards

    Albert
    On 2012/09/07 13:01:40, jsing wrote:
    Hello mailto:golang-dev@googlegroups.com,
    I'd like you to review this change to
    https://go.googlecode.com/hg/
    http://codereview.appspot.com/6501101/
  • Dave at Sep 10, 2012 at 3:43 am
    This looks to have fixed the problem with cgo + enums. I agree a test
    would be good. If you like you can use the goyaml pkg,
    launchpad.net/goyaml/yamlh:153~ as it broke after 41976e2fec9.

    http://codereview.appspot.com/6501101/
  • Russ Cox at Sep 17, 2012 at 8:55 pm
    Is it possible to add a test? See misc/cgo.
  • Jsing at Sep 18, 2012 at 3:44 pm

    On 2012/09/17 20:55:06, rsc wrote:
    Is it possible to add a test? See misc/cgo.
    Unfortunately I've been stuck working on other things. It is possible to
    add a test, however it will be flaky/intermittent at best, since the
    success/failure is dependent on symbol ordering. I'll try to get this
    done in the next day or so.

    https://codereview.appspot.com/6501101/
  • Dave Cheney at Sep 18, 2012 at 4:13 pm
    I can try to write up something tonight by adapting a piece of cgo
    code which is currently failing.
    On Wed, Sep 19, 2012 at 1:44 AM, wrote:
    On 2012/09/17 20:55:06, rsc wrote:

    Is it possible to add a test? See misc/cgo.

    Unfortunately I've been stuck working on other things. It is possible to
    add a test, however it will be flaky/intermittent at best, since the
    success/failure is dependent on symbol ordering. I'll try to get this
    done in the next day or so.

    https://codereview.appspot.com/6501101/
  • Jsing at Sep 19, 2012 at 1:53 pm

    On 2012/09/17 20:55:06, rsc wrote:
    Is it possible to add a test? See misc/cgo.
    Added tests for issues 2470 and 4054.

    PTAL.

    https://codereview.appspot.com/6501101/
  • Dave at Sep 19, 2012 at 2:19 pm

    On 2012/09/19 13:29:57, jsing wrote:
    On 2012/09/17 20:55:06, rsc wrote:
    Is it possible to add a test? See misc/cgo.
    Added tests for issues 2470 and 4054.
    PTAL.
    LGTM

    https://codereview.appspot.com/6501101/
  • Dave at Sep 19, 2012 at 2:19 pm
  • Fullung at Sep 19, 2012 at 4:48 pm
  • Minux Ma at Sep 19, 2012 at 5:07 pm
  • Jsing at Sep 20, 2012 at 3:21 am
    *** Submitted as
    http://code.google.com/p/go/source/detail?r=979c5e5b1308 ***

    cgo: process DWARF info even when debug data is used for value

    Always process the DWARF info, even when the const value is determined
    using the debug data block. This ensures that the injected enum is
    removed and future loads of the same constant do not trigger
    inconsistent definitions.

    Add tests for issues 2470 and 4054.
    Fixes issue 4054.

    R=golang-dev, fullung, dave, rsc, minux.ma
    CC=golang-dev
    http://codereview.appspot.com/6501101


    http://codereview.appspot.com/6501101/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedSep 7, '12 at 1:27p
activeSep 20, '12 at 3:21a
posts12
users5
websitegolang.org

People

Translate

site design / logo © 2022 Grokbase