From 56fc6b76bfd407e89df778f6975e19be698fb09d Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 21 Sep 2020 16:13:00 +0200 Subject: [PATCH 1/5] Small cosmetic changes --- legacy/builder/constants/constants.go | 1 - legacy/builder/phases/libraries_builder.go | 2 +- legacy/builder/phases/linker.go | 4 ++-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/legacy/builder/constants/constants.go b/legacy/builder/constants/constants.go index 8606ec78410..9e2a11f7146 100644 --- a/legacy/builder/constants/constants.go +++ b/legacy/builder/constants/constants.go @@ -31,7 +31,6 @@ const BUILD_PROPERTIES_COMPILER_WARNING_FLAGS = "compiler.warning_flags" const BUILD_PROPERTIES_FQBN = "build.fqbn" const BUILD_PROPERTIES_INCLUDES = "includes" const BUILD_PROPERTIES_OBJECT_FILE = "object_file" -const BUILD_PROPERTIES_OBJECT_FILES = "object_files" const BUILD_PROPERTIES_PATTERN = "pattern" const BUILD_PROPERTIES_PID = "pid" const BUILD_PROPERTIES_PREPROCESSED_FILE_PATH = "preprocessed_file_path" diff --git a/legacy/builder/phases/libraries_builder.go b/legacy/builder/phases/libraries_builder.go index 2a0b0be2dbc..3ebdf7483e3 100644 --- a/legacy/builder/phases/libraries_builder.go +++ b/legacy/builder/phases/libraries_builder.go @@ -112,7 +112,7 @@ func compileLibraries(ctx *types.Context, libraries libraries.List, buildPath *p if err != nil { return nil, errors.WithStack(err) } - objectFiles = append(objectFiles, libraryObjectFiles...) + objectFiles.AddAll(libraryObjectFiles) ctx.Progress.CompleteStep() builder_utils.PrintProgressIfProgressEnabledAndMachineLogger(ctx) diff --git a/legacy/builder/phases/linker.go b/legacy/builder/phases/linker.go index 32bcaf8bafb..41209d26293 100644 --- a/legacy/builder/phases/linker.go +++ b/legacy/builder/phases/linker.go @@ -58,14 +58,14 @@ func (s *Linker) Run(ctx *types.Context) error { func link(ctx *types.Context, objectFiles paths.PathList, coreDotARelPath *paths.Path, coreArchiveFilePath *paths.Path, buildProperties *properties.Map) error { quotedObjectFiles := utils.Map(objectFiles.AsStrings(), wrapWithDoubleQuotes) - objectFileList := strings.Join(quotedObjectFiles, constants.SPACE) + objectFileList := strings.Join(quotedObjectFiles, " ") properties := buildProperties.Clone() properties.Set(constants.BUILD_PROPERTIES_COMPILER_C_ELF_FLAGS, properties.Get(constants.BUILD_PROPERTIES_COMPILER_C_ELF_FLAGS)) properties.Set(constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS, properties.Get(constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS+"."+ctx.WarningsLevel)) properties.Set(constants.BUILD_PROPERTIES_ARCHIVE_FILE, coreDotARelPath.String()) properties.Set(constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH, coreArchiveFilePath.String()) - properties.Set(constants.BUILD_PROPERTIES_OBJECT_FILES, objectFileList) + properties.Set("object_files", objectFileList) _, _, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_C_COMBINE_PATTERN, false /* stdout */, utils.ShowIfVerbose /* stderr */, utils.Show) return err From f20c72e2322ac3d428b12f9c045d0112400573f4 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 27 Oct 2020 00:23:42 +0100 Subject: [PATCH 2/5] Create an archive file if the number of object files is too big This should fix the command line too big issue on Windows. --- legacy/builder/phases/linker.go | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/legacy/builder/phases/linker.go b/legacy/builder/phases/linker.go index 41209d26293..e52a5d4c531 100644 --- a/legacy/builder/phases/linker.go +++ b/legacy/builder/phases/linker.go @@ -57,8 +57,26 @@ func (s *Linker) Run(ctx *types.Context) error { } func link(ctx *types.Context, objectFiles paths.PathList, coreDotARelPath *paths.Path, coreArchiveFilePath *paths.Path, buildProperties *properties.Map) error { - quotedObjectFiles := utils.Map(objectFiles.AsStrings(), wrapWithDoubleQuotes) - objectFileList := strings.Join(quotedObjectFiles, " ") + objectFileList := strings.Join(utils.Map(objectFiles.AsStrings(), wrapWithDoubleQuotes), " ") + + // If command line length is too big (> 30000 chars), try to collect the object files into an archive + // and use that archive to complete the build. + if len(objectFileList) > 30000 { + objectFilesArchive := ctx.BuildPath.Join("object_files.a") + // Recreate the archive at every build + _ = objectFilesArchive.Remove() + properties := buildProperties.Clone() + properties.Set("archive_file", objectFilesArchive.Base()) + properties.SetPath("archive_file_path", objectFilesArchive) + for _, objFile := range objectFiles { + properties.SetPath("object_file", objFile) + _, _, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_AR_PATTERN, false, utils.ShowIfVerbose /* stdout */, utils.Show /* stderr */) + if err != nil { + return err + } + } + objectFileList = "-Wl,--whole-archive " + wrapWithDoubleQuotes(objectFilesArchive.String()) + " -Wl,--no-whole-archive" + } properties := buildProperties.Clone() properties.Set(constants.BUILD_PROPERTIES_COMPILER_C_ELF_FLAGS, properties.Get(constants.BUILD_PROPERTIES_COMPILER_C_ELF_FLAGS)) From 9e038eac00e64be652dd3a62769a8e6781bb7423 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 27 Oct 2020 00:24:35 +0100 Subject: [PATCH 3/5] fixed comments --- legacy/builder/phases/linker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/legacy/builder/phases/linker.go b/legacy/builder/phases/linker.go index e52a5d4c531..26c8cc41162 100644 --- a/legacy/builder/phases/linker.go +++ b/legacy/builder/phases/linker.go @@ -85,7 +85,7 @@ func link(ctx *types.Context, objectFiles paths.PathList, coreDotARelPath *paths properties.Set(constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH, coreArchiveFilePath.String()) properties.Set("object_files", objectFileList) - _, _, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_C_COMBINE_PATTERN, false /* stdout */, utils.ShowIfVerbose /* stderr */, utils.Show) + _, _, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_C_COMBINE_PATTERN, false, utils.ShowIfVerbose /* stdout */, utils.Show /* stderr */) return err } From 81775103cb07648b092fbeaba856a0fe354227d8 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 27 Oct 2020 00:58:20 +0100 Subject: [PATCH 4/5] When exploiting ar-chives make a .a file for each soruce subfolder This is required because gcc-ar checks if an object file is already in the archive by looking ONLY at the filename WITHOUT the path, so it may happens that, for example, an object file named "subdir/spi.o", already inside the archive, may be overwritten by an object file in "anotherdir/spi.o" only because they are both named spi.o and even if they are compiled on different directories. --- legacy/builder/phases/linker.go | 35 +++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/legacy/builder/phases/linker.go b/legacy/builder/phases/linker.go index 26c8cc41162..af27f47625c 100644 --- a/legacy/builder/phases/linker.go +++ b/legacy/builder/phases/linker.go @@ -59,23 +59,38 @@ func (s *Linker) Run(ctx *types.Context) error { func link(ctx *types.Context, objectFiles paths.PathList, coreDotARelPath *paths.Path, coreArchiveFilePath *paths.Path, buildProperties *properties.Map) error { objectFileList := strings.Join(utils.Map(objectFiles.AsStrings(), wrapWithDoubleQuotes), " ") - // If command line length is too big (> 30000 chars), try to collect the object files into an archive - // and use that archive to complete the build. + // If command line length is too big (> 30000 chars), try to collect the object files into archives + // and use that archives to complete the build. if len(objectFileList) > 30000 { - objectFilesArchive := ctx.BuildPath.Join("object_files.a") - // Recreate the archive at every build - _ = objectFilesArchive.Remove() + + // We must create an object file for each visited directory: this is required becuase gcc-ar checks + // if an object file is already in the archive by looking ONLY at the filename WITHOUT the path, so + // it may happen that a subdir/spi.o inside the archive may be overwritten by a anotherdir/spi.o + // because thery are both named spi.o. + properties := buildProperties.Clone() - properties.Set("archive_file", objectFilesArchive.Base()) - properties.SetPath("archive_file_path", objectFilesArchive) - for _, objFile := range objectFiles { - properties.SetPath("object_file", objFile) + archives := map[string]bool{} + for _, object := range objectFiles { + archive := object.Parent().Join("objs.a") + if !archives[archive.String()] { + archives[archive.String()] = true + // Cleanup old archives + _ = archive.Remove() + } + properties.Set("archive_file", archive.Base()) + properties.SetPath("archive_file_path", archive) + properties.SetPath("object_file", object) _, _, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_AR_PATTERN, false, utils.ShowIfVerbose /* stdout */, utils.Show /* stderr */) if err != nil { return err } } - objectFileList = "-Wl,--whole-archive " + wrapWithDoubleQuotes(objectFilesArchive.String()) + " -Wl,--no-whole-archive" + + objectFileList = "" + for archive := range archives { + objectFileList += " " + wrapWithDoubleQuotes(archive) + } + objectFileList = "-Wl,--whole-archive" + objectFileList + " -Wl,--no-whole-archive" } properties := buildProperties.Clone() From d2dfefc11da0d188192e1169e983048eac205cb9 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 27 Oct 2020 17:41:19 +0100 Subject: [PATCH 5/5] using paths.PathList to keep objectFileList --- legacy/builder/phases/linker.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/legacy/builder/phases/linker.go b/legacy/builder/phases/linker.go index af27f47625c..792f854d57e 100644 --- a/legacy/builder/phases/linker.go +++ b/legacy/builder/phases/linker.go @@ -69,11 +69,11 @@ func link(ctx *types.Context, objectFiles paths.PathList, coreDotARelPath *paths // because thery are both named spi.o. properties := buildProperties.Clone() - archives := map[string]bool{} + archives := paths.NewPathList() for _, object := range objectFiles { archive := object.Parent().Join("objs.a") - if !archives[archive.String()] { - archives[archive.String()] = true + if !archives.Contains(archive) { + archives.Add(archive) // Cleanup old archives _ = archive.Remove() } @@ -86,11 +86,8 @@ func link(ctx *types.Context, objectFiles paths.PathList, coreDotARelPath *paths } } - objectFileList = "" - for archive := range archives { - objectFileList += " " + wrapWithDoubleQuotes(archive) - } - objectFileList = "-Wl,--whole-archive" + objectFileList + " -Wl,--no-whole-archive" + objectFileList = strings.Join(utils.Map(archives.AsStrings(), wrapWithDoubleQuotes), " ") + objectFileList = "-Wl,--whole-archive " + objectFileList + " -Wl,--no-whole-archive" } properties := buildProperties.Clone()