| Message ID | 1482241596-31688-2-git-send-email-jezz@sysmic.org |
|---|---|
| State | Accepted |
| Commit | 76838f63412a30a358210e457dda4b79f7730624 |
| Headers | show |
Hi Jérôme, all, On Tue, Dec 20, 2016 at 2:46 PM, Jérôme Pouiller <jezz@sysmic.org> wrote: > The use __DATE__ and __TIME__ are one of most common sources of > non-reproducible binaries. In order to fix that, gcc begin to support > SOURCE_DATE_EPOCH variable. This patch take advantage of toolchain-wrapper > to provide support of SOURCE_DATE_EPOCH to older gcc versions. > > Function get_source_date_epoch() come directly from gcc git. > > This work was sponsored by `BA Robotic Systems'. > > Signed-off-by: Jérôme Pouiller <jezz@sysmic.org> Reviewed-by: Samuel Martin <s.martin49@gmail.com> Regards,
>>>>> "Jérôme" == Jérôme Pouiller <jezz@sysmic.org> writes: > The use __DATE__ and __TIME__ are one of most common sources of The use of. > non-reproducible binaries. In order to fix that, gcc begin to support > SOURCE_DATE_EPOCH variable. This patch take advantage of toolchain-wrapper > to provide support of SOURCE_DATE_EPOCH to older gcc versions. > Function get_source_date_epoch() come directly from gcc git. > This work was sponsored by `BA Robotic Systems'. > Signed-off-by: Jérôme Pouiller <jezz@sysmic.org> > --- > Notes: > v3: > - Handle $SOURCE_DATE_EPOCH at runtime (Thomas) > v2: > - Overload __TIME__ and __DATE__ instead of patching gcc (Thomas) > toolchain/toolchain-wrapper.c | 74 ++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 73 insertions(+), 1 deletion(-) > diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c > index d59629b..6150574 100644 > --- a/toolchain/toolchain-wrapper.c > +++ b/toolchain/toolchain-wrapper.c > @@ -22,12 +22,17 @@ > #include <unistd.h> > #include <stdlib.h> > #include <errno.h> > +#include <time.h> > #ifdef BR_CCACHE > static char ccache_path[PATH_MAX]; > #endif > static char path[PATH_MAX]; > static char sysroot[PATH_MAX]; > +// strlen("-D__TIME__=\"HH:MM:SS\"") + 1 = 22 > +static char source_time[22]; > +// strlen("-D__DATE__=\"MMM DD YYYY\"") + 1 = 25 > +static char source_date[25]; It is nicer to simply use sizeof on that string, E.G.: static char source_time[sizeof("-D__TIME__=\"HH:MM:SS\"")]; Then the comment can be dropped and the comment never gets out of sync with the size. > /** > * GCC errors out with certain combinations of arguments (examples are > @@ -39,8 +44,11 @@ static char sysroot[PATH_MAX]; > * -mfloat-abi= > * -march= > * -mcpu= > + * -D__TIME__= > + * -D__DATE__= > + * -Wno-builtin-macro-redefined > */ > -#define EXCLUSIVE_ARGS 3 > +#define EXCLUSIVE_ARGS 6 > static char *predef_args[] = { > #ifdef BR_CCACHE > @@ -139,6 +147,47 @@ static void check_unsafe_path(const char *arg, > } > } > +/* Read SOURCE_DATE_EPOCH from environment to have a deterministic > + * timestamp to replace embedded current dates to get reproducible > + * results. Returns -1 if SOURCE_DATE_EPOCH is not defined. > + */ > +time_t get_source_date_epoch() This should be static. > +{ > + char *source_date_epoch; > + long long epoch; > + char *endptr; > + > + source_date_epoch = getenv("SOURCE_DATE_EPOCH"); > + if (!source_date_epoch) > + return (time_t) -1; > + > + errno = 0; > + epoch = strtoll (source_date_epoch, &endptr, 10); NIT: No space between strtoll and '('. > + if ((errno == ERANGE && (epoch == LLONG_MAX || epoch == LLONG_MIN)) > + || (errno != 0 && epoch == 0)) { > + fprintf(stderr, "environment variable $SOURCE_DATE_EPOCH: " > + "strtoll: %s\n", strerror(errno)); > + exit(2); > + } > + if (endptr == source_date_epoch) { > + fprintf(stderr, "environment variable $SOURCE_DATE_EPOCH: " > + "no digits were found: %s\n", endptr); > + exit(2); > + } > + if (*endptr != '\0') { > + fprintf(stderr, "environment variable $SOURCE_DATE_EPOCH: " > + "trailing garbage: %s\n", endptr); > + exit(2); > + } > + if (epoch < 0) { > + fprintf(stderr, "environment variable $SOURCE_DATE_EPOCH: " > + "value must be nonnegative: %lld \n", epoch); > + exit(2); > + } I'm not sure this detailed error handling is really needed, but OK. Committed, thanks.
Hello Peter, On Tuesday 07 February 2017 21:41:41 Peter Korsgaard wrote: [...] > > +{ > > + char *source_date_epoch; > > + long long epoch; > > + char *endptr; > > + > > + source_date_epoch = getenv("SOURCE_DATE_EPOCH"); > > + if (!source_date_epoch) > > + return (time_t) -1; > > + > > + errno = 0; > > + epoch = strtoll (source_date_epoch, &endptr, 10); > > NIT: No space between strtoll and '('. > > > + if ((errno == ERANGE && (epoch == LLONG_MAX || epoch == LLONG_MIN)) > > + || (errno != 0 && epoch == 0)) { > > + fprintf(stderr, "environment variable $SOURCE_DATE_EPOCH: " > > + "strtoll: %s\n", strerror(errno)); > > + exit(2); > > + } > > + if (endptr == source_date_epoch) { > > + fprintf(stderr, "environment variable $SOURCE_DATE_EPOCH: " > > + "no digits were found: %s\n", endptr); > > + exit(2); > > + } > > + if (*endptr != '\0') { > > + fprintf(stderr, "environment variable $SOURCE_DATE_EPOCH: " > > + "trailing garbage: %s\n", endptr); > > + exit(2); > > + } > > + if (epoch < 0) { > > + fprintf(stderr, "environment variable $SOURCE_DATE_EPOCH: " > > + "value must be nonnegative: %lld \n", epoch); > > + exit(2); > > + } > > I'm not sure this detailed error handling is really needed, but OK. In fact I copy-paste get_source_date_epoch() from gcc git. So toolchain wrapper will produce exactly same error messages than upstream gcc (and it include same coding style error :-) ).
>>>>> "Jérôme" == Jérôme Pouiller <jezz@sysmic.org> writes: Hi, >> I'm not sure this detailed error handling is really needed, but OK. > In fact I copy-paste get_source_date_epoch() from gcc git. So toolchain > wrapper will produce exactly same error messages than upstream gcc > (and it include same coding style error :-) ). Yes, I noticed that later on as well (which is why I added the reference to gcc git).
Hello, On Wed, 08 Feb 2017 11:07:07 +0100, Jérôme Pouiller wrote: > > I'm not sure this detailed error handling is really needed, but OK. > > In fact I copy-paste get_source_date_epoch() from gcc git. So toolchain > wrapper will produce exactly same error messages than upstream gcc > (and it include same coding style error :-) ). So now our toolchain-wrapper is under GPLv3 :-/ Thomas
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: > Hello, > On Wed, 08 Feb 2017 11:07:07 +0100, Jérôme Pouiller wrote: >> > I'm not sure this detailed error handling is really needed, but OK. >> >> In fact I copy-paste get_source_date_epoch() from gcc git. So toolchain >> wrapper will produce exactly same error messages than upstream gcc >> (and it include same coding style error :-) ). > So now our toolchain-wrapper is under GPLv3 :-/ Yes, if the function is considered large and and original enough to be considered copyrightable - Afterall it is just a getenv + strtoll and a bit of error checking.
On Wednesday 08 February 2017 14:46:09 Peter Korsgaard wrote: > Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: > > On Wed, 08 Feb 2017 11:07:07 +0100, Jérôme Pouiller wrote: > > >> > I'm not sure this detailed error handling is really needed, but OK. > >> > >> In fact I copy-paste get_source_date_epoch() from gcc git. So toolchain > >> wrapper will produce exactly same error messages than upstream gcc > >> (and it include same coding style error :-) ). > > > So now our toolchain-wrapper is under GPLv3 :-/ Indeed, I didn't noticed this problem. > Yes, if the function is considered large and and original enough to be > considered copyrightable - Afterall it is just a getenv + strtoll and a > bit of error checking. I am not a lawyer, but my code is clearly a "derivative work" of get_source_date_epoch() from gcc. What do you prefer? 1. Revert and rewrite function: it is not a big amount of work. 2. Postpone problem: Buildroot also contains patches under GPLv3 and anyway, users will use a GPLv3 compiler.
>>>>> "Jérôme" == Jérôme Pouiller <jezz@sysmic.org> writes: >> Yes, if the function is considered large and and original enough to be >> considered copyrightable - Afterall it is just a getenv + strtoll and a >> bit of error checking. > I am not a lawyer, but my code is clearly a "derivative work" of > get_source_date_epoch() from gcc. Neither am I. I guess the question is if that function contains "substantial amount of creative work" to make it copyrightable. > What do you prefer? > 1. Revert and rewrite function: it is not a big amount of work. But it is hard to claim that your reimplementation isn't a derrived work of get_source_date_epoch() - I mean, there is a limit to how many different ways you can write getenv + strtoll and a bit of error handling. > 2. Postpone problem: Buildroot also contains patches under GPLv3 and > anyway, users will use a GPLv3 compiler. I think that is the most sensible thing to do. What do others say?
Hello, On Wed, 08 Feb 2017 15:29:27 +0100, Peter Korsgaard wrote: > > 2. Postpone problem: Buildroot also contains patches under GPLv3 and > > anyway, users will use a GPLv3 compiler. > > I think that is the most sensible thing to do. What do others say? Indeed, gcc itself, which is called by this wrapper, is under GPLv3. So it's not a big deal if the wrapper is GPLv3 I believe. Thomas
diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c index d59629b..6150574 100644 --- a/toolchain/toolchain-wrapper.c +++ b/toolchain/toolchain-wrapper.c @@ -22,12 +22,17 @@ #include <unistd.h> #include <stdlib.h> #include <errno.h> +#include <time.h> #ifdef BR_CCACHE static char ccache_path[PATH_MAX]; #endif static char path[PATH_MAX]; static char sysroot[PATH_MAX]; +// strlen("-D__TIME__=\"HH:MM:SS\"") + 1 = 22 +static char source_time[22]; +// strlen("-D__DATE__=\"MMM DD YYYY\"") + 1 = 25 +static char source_date[25]; /** * GCC errors out with certain combinations of arguments (examples are @@ -39,8 +44,11 @@ static char sysroot[PATH_MAX]; * -mfloat-abi= * -march= * -mcpu= + * -D__TIME__= + * -D__DATE__= + * -Wno-builtin-macro-redefined */ -#define EXCLUSIVE_ARGS 3 +#define EXCLUSIVE_ARGS 6 static char *predef_args[] = { #ifdef BR_CCACHE @@ -139,6 +147,47 @@ static void check_unsafe_path(const char *arg, } } +/* Read SOURCE_DATE_EPOCH from environment to have a deterministic + * timestamp to replace embedded current dates to get reproducible + * results. Returns -1 if SOURCE_DATE_EPOCH is not defined. + */ +time_t get_source_date_epoch() +{ + char *source_date_epoch; + long long epoch; + char *endptr; + + source_date_epoch = getenv("SOURCE_DATE_EPOCH"); + if (!source_date_epoch) + return (time_t) -1; + + errno = 0; + epoch = strtoll (source_date_epoch, &endptr, 10); + if ((errno == ERANGE && (epoch == LLONG_MAX || epoch == LLONG_MIN)) + || (errno != 0 && epoch == 0)) { + fprintf(stderr, "environment variable $SOURCE_DATE_EPOCH: " + "strtoll: %s\n", strerror(errno)); + exit(2); + } + if (endptr == source_date_epoch) { + fprintf(stderr, "environment variable $SOURCE_DATE_EPOCH: " + "no digits were found: %s\n", endptr); + exit(2); + } + if (*endptr != '\0') { + fprintf(stderr, "environment variable $SOURCE_DATE_EPOCH: " + "trailing garbage: %s\n", endptr); + exit(2); + } + if (epoch < 0) { + fprintf(stderr, "environment variable $SOURCE_DATE_EPOCH: " + "value must be nonnegative: %lld \n", epoch); + exit(2); + } + + return (time_t) epoch; +} + int main(int argc, char **argv) { char **args, **cur, **exec_args; @@ -149,6 +198,7 @@ int main(int argc, char **argv) char *paranoid_wrapper; int paranoid; int ret, i, count = 0, debug; + time_t source_date_epoch; /* Calculate the relative paths */ basename = strrchr(progpath, '/'); @@ -254,6 +304,28 @@ int main(int argc, char **argv) } #endif /* ARCH || CPU */ + source_date_epoch = get_source_date_epoch(); + if (source_date_epoch != -1) { + struct tm *tm = localtime(&source_date_epoch); + if (!tm) { + perror("__FILE__: localtime"); + return 3; + } + ret = strftime(source_time, sizeof(source_time), "-D__TIME__=\"%T\"", tm); + if (!ret) { + perror("__FILE__: overflow"); + return 3; + } + *cur++ = source_time; + ret = strftime(source_date, sizeof(source_date), "-D__DATE__=\"%b %e %Y\"", tm); + if (!ret) { + perror("__FILE__: overflow"); + return 3; + } + *cur++ = source_date; + *cur++ = "-Wno-builtin-macro-redefined"; + } + paranoid_wrapper = getenv("BR_COMPILER_PARANOID_UNSAFE_PATH"); if (paranoid_wrapper && strlen(paranoid_wrapper) > 0) paranoid = 1;
The use __DATE__ and __TIME__ are one of most common sources of non-reproducible binaries. In order to fix that, gcc begin to support SOURCE_DATE_EPOCH variable. This patch take advantage of toolchain-wrapper to provide support of SOURCE_DATE_EPOCH to older gcc versions. Function get_source_date_epoch() come directly from gcc git. This work was sponsored by `BA Robotic Systems'. Signed-off-by: Jérôme Pouiller <jezz@sysmic.org> --- Notes: v3: - Handle $SOURCE_DATE_EPOCH at runtime (Thomas) v2: - Overload __TIME__ and __DATE__ instead of patching gcc (Thomas) toolchain/toolchain-wrapper.c | 74 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-)