* [Tarantool-patches] [PATCH] decimal: fix build with GCC 10
@ 2020-06-05 14:01 Serge Petrenko
2020-06-06 5:31 ` Alexander V. Tikhonov
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Serge Petrenko @ 2020-06-05 14:01 UTC (permalink / raw)
To: avtikhon, alexander.turenko; +Cc: tarantool-patches
GCC 10 produces the following error:
cc1: warning: function may return address of local variable [-Wreturn-local-addr]
Fix it.
Part-of #4966
---
https://github.com/tarantool/tarantool/issues/4966
https://github.com/tarantool/tarantool/tree/sp/gh-4966-decimal-build
src/lib/core/decimal.c | 8 ++++----
src/lib/core/decimal.h | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
index c44b2f6a2..019c68338 100644
--- a/src/lib/core/decimal.c
+++ b/src/lib/core/decimal.c
@@ -176,24 +176,24 @@ decimal_to_integer(decimal_t *dec)
return decimal_check_status(dec, &decimal_context);
}
-decimal_t *
+const decimal_t *
decimal_to_int64(const decimal_t *dec, int64_t *num)
{
decimal_t d = *dec;
if (decimal_to_integer(&d) == NULL)
return NULL;
*num = decNumberToInt64(&d, &decimal_context);
- return decimal_check_status(&d, &decimal_context);
+ return decimal_check_status(&d, &decimal_context) != NULL ? dec : NULL;
}
-decimal_t *
+const decimal_t *
decimal_to_uint64(const decimal_t *dec, uint64_t *num)
{
decimal_t d = *dec;
if (decimal_to_integer(&d) == NULL)
return NULL;
*num = decNumberToUInt64(&d, &decimal_context);
- return decimal_check_status(&d, &decimal_context);
+ return decimal_check_status(&d, &decimal_context) != NULL ? dec : NULL;
}
int
diff --git a/src/lib/core/decimal.h b/src/lib/core/decimal.h
index 9a162c73c..e276344d2 100644
--- a/src/lib/core/decimal.h
+++ b/src/lib/core/decimal.h
@@ -110,11 +110,11 @@ decimal_to_string(const decimal_t *dec);
* \param[out] num - the result
* @return NULL if \a dec doesn't fit into int64_t
*/
-decimal_t *
+const decimal_t *
decimal_to_int64(const decimal_t *dec, int64_t *num);
/** \sa decimal_to_int64 */
-decimal_t *
+const decimal_t *
decimal_to_uint64(const decimal_t *dec, uint64_t *num);
/**
--
2.24.3 (Apple Git-128)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Tarantool-patches] [PATCH] decimal: fix build with GCC 10
2020-06-05 14:01 [Tarantool-patches] [PATCH] decimal: fix build with GCC 10 Serge Petrenko
@ 2020-06-06 5:31 ` Alexander V. Tikhonov
2020-06-08 10:23 ` Kirill Yukhin
2020-06-08 11:22 ` Alexander Turenko
2 siblings, 0 replies; 6+ messages in thread
From: Alexander V. Tikhonov @ 2020-06-06 5:31 UTC (permalink / raw)
To: Serge Petrenko; +Cc: tarantool-patches
Hi Sergey, thanks a lot for the patch, I've successfully checked it
on Fedora 32 and on the full testing process:
https://gitlab.com/tarantool/tarantool/-/pipelines/153432554
On Fri, Jun 05, 2020 at 05:01:25PM +0300, Serge Petrenko wrote:
> GCC 10 produces the following error:
> cc1: warning: function may return address of local variable [-Wreturn-local-addr]
>
> Fix it.
>
> Part-of #4966
> ---
> https://github.com/tarantool/tarantool/issues/4966
> https://github.com/tarantool/tarantool/tree/sp/gh-4966-decimal-build
>
> src/lib/core/decimal.c | 8 ++++----
> src/lib/core/decimal.h | 4 ++--
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
> index c44b2f6a2..019c68338 100644
> --- a/src/lib/core/decimal.c
> +++ b/src/lib/core/decimal.c
> @@ -176,24 +176,24 @@ decimal_to_integer(decimal_t *dec)
> return decimal_check_status(dec, &decimal_context);
> }
>
> -decimal_t *
> +const decimal_t *
> decimal_to_int64(const decimal_t *dec, int64_t *num)
> {
> decimal_t d = *dec;
> if (decimal_to_integer(&d) == NULL)
> return NULL;
> *num = decNumberToInt64(&d, &decimal_context);
> - return decimal_check_status(&d, &decimal_context);
> + return decimal_check_status(&d, &decimal_context) != NULL ? dec : NULL;
> }
>
> -decimal_t *
> +const decimal_t *
> decimal_to_uint64(const decimal_t *dec, uint64_t *num)
> {
> decimal_t d = *dec;
> if (decimal_to_integer(&d) == NULL)
> return NULL;
> *num = decNumberToUInt64(&d, &decimal_context);
> - return decimal_check_status(&d, &decimal_context);
> + return decimal_check_status(&d, &decimal_context) != NULL ? dec : NULL;
> }
>
> int
> diff --git a/src/lib/core/decimal.h b/src/lib/core/decimal.h
> index 9a162c73c..e276344d2 100644
> --- a/src/lib/core/decimal.h
> +++ b/src/lib/core/decimal.h
> @@ -110,11 +110,11 @@ decimal_to_string(const decimal_t *dec);
> * \param[out] num - the result
> * @return NULL if \a dec doesn't fit into int64_t
> */
> -decimal_t *
> +const decimal_t *
> decimal_to_int64(const decimal_t *dec, int64_t *num);
>
> /** \sa decimal_to_int64 */
> -decimal_t *
> +const decimal_t *
> decimal_to_uint64(const decimal_t *dec, uint64_t *num);
>
> /**
> --
> 2.24.3 (Apple Git-128)
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Tarantool-patches] [PATCH] decimal: fix build with GCC 10
2020-06-05 14:01 [Tarantool-patches] [PATCH] decimal: fix build with GCC 10 Serge Petrenko
2020-06-06 5:31 ` Alexander V. Tikhonov
@ 2020-06-08 10:23 ` Kirill Yukhin
2020-06-08 11:22 ` Alexander Turenko
2 siblings, 0 replies; 6+ messages in thread
From: Kirill Yukhin @ 2020-06-08 10:23 UTC (permalink / raw)
To: Serge Petrenko; +Cc: tarantool-patches, alexander.turenko
Hello,
On 05 июн 17:01, Serge Petrenko wrote:
> GCC 10 produces the following error:
> cc1: warning: function may return address of local variable [-Wreturn-local-addr]
>
> Fix it.
>
> Part-of #4966
> ---
> https://github.com/tarantool/tarantool/issues/4966
> https://github.com/tarantool/tarantool/tree/sp/gh-4966-decimal-build
LGTM.
I've checked your patch into 2.3, 2.4 and master.
--
Regards, Kirill Yukhin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH] decimal: fix build with GCC 10
2020-06-05 14:01 [Tarantool-patches] [PATCH] decimal: fix build with GCC 10 Serge Petrenko
2020-06-06 5:31 ` Alexander V. Tikhonov
2020-06-08 10:23 ` Kirill Yukhin
@ 2020-06-08 11:22 ` Alexander Turenko
2020-06-08 11:30 ` Serge Petrenko
2 siblings, 1 reply; 6+ messages in thread
From: Alexander Turenko @ 2020-06-08 11:22 UTC (permalink / raw)
To: Serge Petrenko; +Cc: tarantool-patches
The patch itself is okay.
On Fri, Jun 05, 2020 at 05:01:25PM +0300, Serge Petrenko wrote:
> GCC 10 produces the following error:
> cc1: warning: function may return address of local variable [-Wreturn-local-addr]
>
> Fix it.
It would be good to mention whether it is a real problem or
false-positive. (The patch is already pushed, so this comment is just
for information.)
As I see, we actually return a data on the stack before the patch, so it
worth to mention the bugfix in the release notes. Kirill, can you add
this?
@ChangeLog
- Fix use-after-scope when converting a decimal number to int64_t or
uint64_t that may lead to undefined behaviour in tuple comparators and
so in behaviour of space indices (part of gh-4966).
(Serge, correct me if I'm wrong here.)
BTW, are those hints pesisted for vinyl? Can we get wrong results even
after upgrade?
WBR, Alexander Turenko.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH] decimal: fix build with GCC 10
2020-06-08 11:22 ` Alexander Turenko
@ 2020-06-08 11:30 ` Serge Petrenko
2020-06-08 11:36 ` Alexander Turenko
0 siblings, 1 reply; 6+ messages in thread
From: Serge Petrenko @ 2020-06-08 11:30 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
08.06.2020 14:22, Alexander Turenko пишет:
> The patch itself is okay.
>
> On Fri, Jun 05, 2020 at 05:01:25PM +0300, Serge Petrenko wrote:
>> GCC 10 produces the following error:
>> cc1: warning: function may return address of local variable [-Wreturn-local-addr]
>>
>> Fix it.
> It would be good to mention whether it is a real problem or
> false-positive. (The patch is already pushed, so this comment is just
> for information.)
It's a false-positive. The return value is only used to test `dec !=
NULL`.
`dec == NULL` means an error.So, no UB here.
I guess no need for a changelog entry then.
>
> As I see, we actually return a data on the stack before the patch, so it
> worth to mention the bugfix in the release notes. Kirill, can you add
> this?
>
> @ChangeLog
>
> - Fix use-after-scope when converting a decimal number to int64_t or
> uint64_t that may lead to undefined behaviour in tuple comparators and
> so in behaviour of space indices (part of gh-4966).
>
> (Serge, correct me if I'm wrong here.
>
> BTW, are those hints pesisted for vinyl? Can we get wrong results even
> after upgrade?
>
> WBR, Alexander Turenko.
--
Serge Petrenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH] decimal: fix build with GCC 10
2020-06-08 11:30 ` Serge Petrenko
@ 2020-06-08 11:36 ` Alexander Turenko
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Turenko @ 2020-06-08 11:36 UTC (permalink / raw)
To: Serge Petrenko; +Cc: tarantool-patches
On Mon, Jun 08, 2020 at 02:30:43PM +0300, Serge Petrenko wrote:
>
> 08.06.2020 14:22, Alexander Turenko пишет:
> > The patch itself is okay.
> >
> > On Fri, Jun 05, 2020 at 05:01:25PM +0300, Serge Petrenko wrote:
> > > GCC 10 produces the following error:
> > > cc1: warning: function may return address of local variable [-Wreturn-local-addr]
> > >
> > > Fix it.
> > It would be good to mention whether it is a real problem or
> > false-positive. (The patch is already pushed, so this comment is just
> > for information.)
>
>
> It's a false-positive. The return value is only used to test `dec !=
> NULL`.
>
> `dec == NULL` means an error.So, no UB here.
>
> I guess no need for a changelog entry then.
Ouch, I see. Disregard so.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-08 11:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-05 14:01 [Tarantool-patches] [PATCH] decimal: fix build with GCC 10 Serge Petrenko
2020-06-06 5:31 ` Alexander V. Tikhonov
2020-06-08 10:23 ` Kirill Yukhin
2020-06-08 11:22 ` Alexander Turenko
2020-06-08 11:30 ` Serge Petrenko
2020-06-08 11:36 ` Alexander Turenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox