* Re: [OE-core] [PATCH v3][master] go: Update reproducibility patch to fix panic errors
2022-12-05 19:49 ` [OE-core] [PATCH v3][master] go: Update reproducibility patch to fix panic errors Ryan Eatmon
@ 2022-12-05 22:16 ` Alexandre Belloni
2022-12-05 22:31 ` Richard Purdie
2023-01-05 12:56 ` Jose Quaresma
2 siblings, 0 replies; 4+ messages in thread
From: Alexandre Belloni @ 2022-12-05 22:16 UTC (permalink / raw)
To: reatmon; +Cc: raj.khem, openembedded-core
Hello,
On 05/12/2022 13:49:48-0600, Ryan Eatmon via lists.openembedded.org wrote:
>
> This might have gotten lost in the fun of the YPS and the many discussions
> around the previous versions of then patch. Was there any more feedback or
> changes that folks want me to make for this patch, or is it good to go?
> Just curious if there was more that you needed from me?
>
It passed successfully on the autobuilders, it is now up to Richard to
merge.
>
> On 11/29/2022 8:12, Ryan Eatmon via lists.openembedded.org wrote:
> > Based on a discussion on the mailing list [1], there are panic
> > errors that occur on a few platforms caused by the patch. We
> > cannot simply remove the original patch due to the
> > reproducibility issues that it addresses, so this patch on the
> > original patch fixes the cause of the panic errors.
> >
> > The previous version of this patch was a little too aggressive
> > in cleaning up the environment. Some of the variables impacted
> > by the filerCompilerFlags() function require at least one value
> > to remain in the array. In this case, the values for ccExe,
> > cxxExe, and fcExe require a value or later code that access
> > them result in a panic related to accessing a value out of range.
> >
> > This updated patch adds a flag that requires keeping the first
> > value so that at least one thing remains and the assignments
> > for the Exes set that flag to true. The first item in the
> > array should be the executable name, so leaving it should be
> > safe.
> >
> > I have run the oe-selftest and everything passed in my setup.
> >
> > There is a bug report [2] filed for the issue that this patch
> > addresses.
> >
> > [YOCTO #14976]
> >
> > [1] https://lists.openembedded.org/g/openembedded-core/topic/94022663
> > [2] https://bugzilla.yoctoproject.org/show_bug.cgi?id=14976
> >
> > Signed-off-by: Ryan Eatmon <reatmon@ti.com>
> > ---
> > v3: Added [YOCTO #14976] to commit message.
> > v2: Added more background and bug report link to commit message.
> >
> > ...ent-based-hash-generation-less-pedan.patch | 30 ++++++++++---------
> > 1 file changed, 16 insertions(+), 14 deletions(-)
> >
> > diff --git a/meta/recipes-devtools/go/go/0001-cmd-go-make-content-based-hash-generation-less-pedan.patch b/meta/recipes-devtools/go/go/0001-cmd-go-make-content-based-hash-generation-less-pedan.patch
> > index 17fa9d9831..43be5cd2e8 100644
> > --- a/meta/recipes-devtools/go/go/0001-cmd-go-make-content-based-hash-generation-less-pedan.patch
> > +++ b/meta/recipes-devtools/go/go/0001-cmd-go-make-content-based-hash-generation-less-pedan.patch
> > @@ -74,7 +74,7 @@ index c88b315..a06455c 100644
> > + cppflags, cflags, cxxflags, fflags, ldflags, _ := b.CFlags(p, true)
> > - ccExe := b.ccExe()
> > -+ ccExe := filterCompilerFlags(b.ccExe())
> > ++ ccExe := filterCompilerFlags(b.ccExe(), true)
> > fmt.Fprintf(h, "CC=%q %q %q %q\n", ccExe, cppflags, cflags, ldflags)
> > // Include the C compiler tool ID so that if the C
> > // compiler changes we rebuild the package.
> > @@ -83,7 +83,7 @@ index c88b315..a06455c 100644
> > }
> > if len(p.CXXFiles)+len(p.SwigCXXFiles) > 0 {
> > - cxxExe := b.cxxExe()
> > -+ cxxExe := filterCompilerFlags(b.cxxExe())
> > ++ cxxExe := filterCompilerFlags(b.cxxExe(), true)
> > fmt.Fprintf(h, "CXX=%q %q\n", cxxExe, cxxflags)
> > if cxxID, err := b.gccToolID(cxxExe[0], "c++"); err == nil {
> > fmt.Fprintf(h, "CXX ID=%q\n", cxxID)
> > @@ -91,7 +91,7 @@ index c88b315..a06455c 100644
> > }
> > if len(p.FFiles) > 0 {
> > - fcExe := b.fcExe()
> > -+ fcExe := filterCompilerFlags(b.fcExe())
> > ++ fcExe := filterCompilerFlags(b.fcExe(), true)
> > fmt.Fprintf(h, "FC=%q %q\n", fcExe, fflags)
> > if fcID, err := b.gccToolID(fcExe[0], "f95"); err == nil {
> > fmt.Fprintf(h, "FC ID=%q\n", fcID)
> > @@ -104,20 +104,22 @@ index c88b315..a06455c 100644
> > }
> > // Configuration specific to compiler toolchain.
> > -@@ -2705,8 +2707,23 @@ func envList(key, def string) []string {
> > +@@ -2705,8 +2707,25 @@ func envList(key, def string) []string {
> > return args
> > }
> > +var filterFlags = os.Getenv("CGO_PEDANTIC") == ""
> > +
> > -+func filterCompilerFlags(flags []string) []string {
> > ++func filterCompilerFlags(flags []string, keepfirst bool) []string {
> > + var newflags []string
> > ++ var realkeepfirst bool = keepfirst
> > + if !filterFlags {
> > + return flags
> > + }
> > + for _, flag := range flags {
> > -+ if strings.HasPrefix(flag, "-m") {
> > ++ if strings.HasPrefix(flag, "-m") || realkeepfirst {
> > + newflags = append(newflags, flag)
> > ++ realkeepfirst = false
> > + }
> > + }
> > + return newflags
> > @@ -129,21 +131,21 @@ index c88b315..a06455c 100644
> > defaults := "-g -O2"
> > if cppflags, err = buildFlags("CPPFLAGS", "", p.CgoCPPFLAGS, checkCompilerFlags); err != nil {
> > -@@ -2724,6 +2741,13 @@ func (b *Builder) CFlags(p *load.Package) (cppflags, cflags, cxxflags, fflags, l
> > +@@ -2724,6 +2743,13 @@ func (b *Builder) CFlags(p *load.Package) (cppflags, cflags, cxxflags, fflags, l
> > if ldflags, err = buildFlags("LDFLAGS", defaults, p.CgoLDFLAGS, checkLinkerFlags); err != nil {
> > return
> > }
> > + if filtered {
> > -+ cppflags = filterCompilerFlags(cppflags)
> > -+ cflags = filterCompilerFlags(cflags)
> > -+ cxxflags = filterCompilerFlags(cxxflags)
> > -+ fflags = filterCompilerFlags(fflags)
> > -+ ldflags = filterCompilerFlags(ldflags)
> > ++ cppflags = filterCompilerFlags(cppflags, false)
> > ++ cflags = filterCompilerFlags(cflags, false)
> > ++ cxxflags = filterCompilerFlags(cxxflags, false)
> > ++ fflags = filterCompilerFlags(fflags, false)
> > ++ ldflags = filterCompilerFlags(ldflags, false)
> > + }
> > return
> > }
> > -@@ -2739,7 +2763,7 @@ var cgoRe = lazyregexp.New(`[/\\:]`)
> > +@@ -2739,7 +2765,7 @@ var cgoRe = lazyregexp.New(`[/\\:]`)
> > func (b *Builder) cgo(a *Action, cgoExe, objdir string, pcCFLAGS, pcLDFLAGS, cgofiles, gccfiles, gxxfiles, mfiles, ffiles []string) (outGo, outObj []string, err error) {
> > p := a.Package
> > @@ -152,7 +154,7 @@ index c88b315..a06455c 100644
> > if err != nil {
> > return nil, nil, err
> > }
> > -@@ -3246,7 +3270,7 @@ func (b *Builder) swigIntSize(objdir string) (intsize string, err error) {
> > +@@ -3246,7 +3272,7 @@ func (b *Builder) swigIntSize(objdir string) (intsize string, err error) {
> > // Run SWIG on one SWIG input file.
> > func (b *Builder) swigOne(a *Action, p *load.Package, file, objdir string, pcCFLAGS []string, cxx bool, intgosize string) (outGo, outC string, err error) {
> >
> >
> >
> >
> >
>
> --
> Ryan Eatmon reatmon@ti.com
> -----------------------------------------
> Texas Instruments, Inc. - LCPD - MGTS
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#174301): https://lists.openembedded.org/g/openembedded-core/message/174301
> Mute This Topic: https://lists.openembedded.org/mt/95334740/3617179
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [OE-core] [PATCH v3][master] go: Update reproducibility patch to fix panic errors
2022-12-05 19:49 ` [OE-core] [PATCH v3][master] go: Update reproducibility patch to fix panic errors Ryan Eatmon
2022-12-05 22:16 ` Alexandre Belloni
@ 2022-12-05 22:31 ` Richard Purdie
2023-01-05 12:56 ` Jose Quaresma
2 siblings, 0 replies; 4+ messages in thread
From: Richard Purdie @ 2022-12-05 22:31 UTC (permalink / raw)
To: reatmon, raj.khem, openembedded-core
On Mon, 2022-12-05 at 13:49 -0600, Ryan Eatmon via
lists.openembedded.org wrote:
> This might have gotten lost in the fun of the YPS and the many
> discussions around the previous versions of then patch. Was there any
> more feedback or changes that folks want me to make for this patch, or
> is it good to go? Just curious if there was more that you needed from me?
I've merged it but I am worried there is a deeper problem here we could
really do with solving properly. We should probably at least have an
open bug about resolving the issue the patch addresses in a better way.
Cheers,
Richard
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [OE-core] [PATCH v3][master] go: Update reproducibility patch to fix panic errors
2022-12-05 19:49 ` [OE-core] [PATCH v3][master] go: Update reproducibility patch to fix panic errors Ryan Eatmon
2022-12-05 22:16 ` Alexandre Belloni
2022-12-05 22:31 ` Richard Purdie
@ 2023-01-05 12:56 ` Jose Quaresma
2 siblings, 0 replies; 4+ messages in thread
From: Jose Quaresma @ 2023-01-05 12:56 UTC (permalink / raw)
To: reatmon; +Cc: raj.khem, openembedded-core
[-- Attachment #1: Type: text/plain, Size: 8245 bytes --]
Hi Ryan,
I am building with this patch and it fixes the problem in my machines,
riscv and am6xxx.
Thanks for fixing this issue.
Jose
Ryan Eatmon via lists.openembedded.org <reatmon=
ti.com@lists.openembedded.org> escreveu no dia segunda, 5/12/2022 à(s)
19:49:
>
> This might have gotten lost in the fun of the YPS and the many
> discussions around the previous versions of then patch. Was there any
> more feedback or changes that folks want me to make for this patch, or
> is it good to go? Just curious if there was more that you needed from me?
>
>
> On 11/29/2022 8:12, Ryan Eatmon via lists.openembedded.org wrote:
> > Based on a discussion on the mailing list [1], there are panic
> > errors that occur on a few platforms caused by the patch. We
> > cannot simply remove the original patch due to the
> > reproducibility issues that it addresses, so this patch on the
> > original patch fixes the cause of the panic errors.
> >
> > The previous version of this patch was a little too aggressive
> > in cleaning up the environment. Some of the variables impacted
> > by the filerCompilerFlags() function require at least one value
> > to remain in the array. In this case, the values for ccExe,
> > cxxExe, and fcExe require a value or later code that access
> > them result in a panic related to accessing a value out of range.
> >
> > This updated patch adds a flag that requires keeping the first
> > value so that at least one thing remains and the assignments
> > for the Exes set that flag to true. The first item in the
> > array should be the executable name, so leaving it should be
> > safe.
> >
> > I have run the oe-selftest and everything passed in my setup.
> >
> > There is a bug report [2] filed for the issue that this patch
> > addresses.
> >
> > [YOCTO #14976]
> >
> > [1] https://lists.openembedded.org/g/openembedded-core/topic/94022663
> > [2] https://bugzilla.yoctoproject.org/show_bug.cgi?id=14976
> >
> > Signed-off-by: Ryan Eatmon <reatmon@ti.com>
> > ---
> > v3: Added [YOCTO #14976] to commit message.
> > v2: Added more background and bug report link to commit message.
> >
> > ...ent-based-hash-generation-less-pedan.patch | 30 ++++++++++---------
> > 1 file changed, 16 insertions(+), 14 deletions(-)
> >
> > diff --git
> a/meta/recipes-devtools/go/go/0001-cmd-go-make-content-based-hash-generation-less-pedan.patch
> b/meta/recipes-devtools/go/go/0001-cmd-go-make-content-based-hash-generation-less-pedan.patch
> > index 17fa9d9831..43be5cd2e8 100644
> > ---
> a/meta/recipes-devtools/go/go/0001-cmd-go-make-content-based-hash-generation-less-pedan.patch
> > +++
> b/meta/recipes-devtools/go/go/0001-cmd-go-make-content-based-hash-generation-less-pedan.patch
> > @@ -74,7 +74,7 @@ index c88b315..a06455c 100644
> > + cppflags, cflags, cxxflags, fflags, ldflags, _ :=
> b.CFlags(p, true)
> >
> > - ccExe := b.ccExe()
> > -+ ccExe := filterCompilerFlags(b.ccExe())
> > ++ ccExe := filterCompilerFlags(b.ccExe(), true)
> > fmt.Fprintf(h, "CC=%q %q %q %q\n", ccExe, cppflags,
> cflags, ldflags)
> > // Include the C compiler tool ID so that if the C
> > // compiler changes we rebuild the package.
> > @@ -83,7 +83,7 @@ index c88b315..a06455c 100644
> > }
> > if len(p.CXXFiles)+len(p.SwigCXXFiles) > 0 {
> > - cxxExe := b.cxxExe()
> > -+ cxxExe := filterCompilerFlags(b.cxxExe())
> > ++ cxxExe := filterCompilerFlags(b.cxxExe(), true)
> > fmt.Fprintf(h, "CXX=%q %q\n", cxxExe, cxxflags)
> > if cxxID, err := b.gccToolID(cxxExe[0], "c++");
> err == nil {
> > fmt.Fprintf(h, "CXX ID=%q\n", cxxID)
> > @@ -91,7 +91,7 @@ index c88b315..a06455c 100644
> > }
> > if len(p.FFiles) > 0 {
> > - fcExe := b.fcExe()
> > -+ fcExe := filterCompilerFlags(b.fcExe())
> > ++ fcExe := filterCompilerFlags(b.fcExe(), true)
> > fmt.Fprintf(h, "FC=%q %q\n", fcExe, fflags)
> > if fcID, err := b.gccToolID(fcExe[0], "f95"); err
> == nil {
> > fmt.Fprintf(h, "FC ID=%q\n", fcID)
> > @@ -104,20 +104,22 @@ index c88b315..a06455c 100644
> > }
> >
> > // Configuration specific to compiler toolchain.
> > -@@ -2705,8 +2707,23 @@ func envList(key, def string) []string {
> > +@@ -2705,8 +2707,25 @@ func envList(key, def string) []string {
> > return args
> > }
> >
> > +var filterFlags = os.Getenv("CGO_PEDANTIC") == ""
> > +
> > -+func filterCompilerFlags(flags []string) []string {
> > ++func filterCompilerFlags(flags []string, keepfirst bool) []string {
> > + var newflags []string
> > ++ var realkeepfirst bool = keepfirst
> > + if !filterFlags {
> > + return flags
> > + }
> > + for _, flag := range flags {
> > -+ if strings.HasPrefix(flag, "-m") {
> > ++ if strings.HasPrefix(flag, "-m") || realkeepfirst {
> > + newflags = append(newflags, flag)
> > ++ realkeepfirst = false
> > + }
> > + }
> > + return newflags
> > @@ -129,21 +131,21 @@ index c88b315..a06455c 100644
> > defaults := "-g -O2"
> >
> > if cppflags, err = buildFlags("CPPFLAGS", "", p.CgoCPPFLAGS,
> checkCompilerFlags); err != nil {
> > -@@ -2724,6 +2741,13 @@ func (b *Builder) CFlags(p *load.Package)
> (cppflags, cflags, cxxflags, fflags, l
> > +@@ -2724,6 +2743,13 @@ func (b *Builder) CFlags(p *load.Package)
> (cppflags, cflags, cxxflags, fflags, l
> > if ldflags, err = buildFlags("LDFLAGS", defaults, p.CgoLDFLAGS,
> checkLinkerFlags); err != nil {
> > return
> > }
> > + if filtered {
> > -+ cppflags = filterCompilerFlags(cppflags)
> > -+ cflags = filterCompilerFlags(cflags)
> > -+ cxxflags = filterCompilerFlags(cxxflags)
> > -+ fflags = filterCompilerFlags(fflags)
> > -+ ldflags = filterCompilerFlags(ldflags)
> > ++ cppflags = filterCompilerFlags(cppflags, false)
> > ++ cflags = filterCompilerFlags(cflags, false)
> > ++ cxxflags = filterCompilerFlags(cxxflags, false)
> > ++ fflags = filterCompilerFlags(fflags, false)
> > ++ ldflags = filterCompilerFlags(ldflags, false)
> > + }
> >
> > return
> > }
> > -@@ -2739,7 +2763,7 @@ var cgoRe = lazyregexp.New(`[/\\:]`)
> > +@@ -2739,7 +2765,7 @@ var cgoRe = lazyregexp.New(`[/\\:]`)
> >
> > func (b *Builder) cgo(a *Action, cgoExe, objdir string, pcCFLAGS,
> pcLDFLAGS, cgofiles, gccfiles, gxxfiles, mfiles, ffiles []string) (outGo,
> outObj []string, err error) {
> > p := a.Package
> > @@ -152,7 +154,7 @@ index c88b315..a06455c 100644
> > if err != nil {
> > return nil, nil, err
> > }
> > -@@ -3246,7 +3270,7 @@ func (b *Builder) swigIntSize(objdir string)
> (intsize string, err error) {
> > +@@ -3246,7 +3272,7 @@ func (b *Builder) swigIntSize(objdir string)
> (intsize string, err error) {
> >
> > // Run SWIG on one SWIG input file.
> > func (b *Builder) swigOne(a *Action, p *load.Package, file, objdir
> string, pcCFLAGS []string, cxx bool, intgosize string) (outGo, outC string,
> err error) {
> >
> >
> >
> >
> >
>
> --
> Ryan Eatmon reatmon@ti.com
> -----------------------------------------
> Texas Instruments, Inc. - LCPD - MGTS
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#174301):
> https://lists.openembedded.org/g/openembedded-core/message/174301
> Mute This Topic: https://lists.openembedded.org/mt/95334740/5052612
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> quaresma.jose@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
--
Best regards,
José Quaresma
[-- Attachment #2: Type: text/html, Size: 10905 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread