Re: CVS commit: src/lib/libc/regex

2018-02-24 Thread Christos Zoulas
On Feb 25, 12:39am, n...@gmx.com (Kamil Rytarowski) wrote:
-- Subject: Re: CVS commit: src/lib/libc/regex

| 
| --MIMEStream=_0+26969_51985210222325_05798576868
| Content-Type: multipart/signed; micalg=pgp-sha256;
|  protocol="application/pgp-signature";
|  boundary="PkRH582jLcQBCd2EFFVVmMFMcjasCXI54"
| 
| This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
| --PkRH582jLcQBCd2EFFVVmMFMcjasCXI54
| Content-Type: multipart/mixed; boundary="9pUp6Nh9t4hLApFrwb0ID8pUSOtt8VKmJ";
|  protected-headers="v1"
| From: Kamil Rytarowski 
| To: source-changes-d@NetBSD.org, Christos Zoulas 
| Message-ID: <23a830fa-8063-e100-c63b-eb86ef7a2...@gmx.com>
| Subject: Re: CVS commit: src/lib/libc/regex
| References: <20160114204147.ba169f...@cvs.netbsd.org>
| In-Reply-To: <20160114204147.ba169f...@cvs.netbsd.org>
| 
| 
| --9pUp6Nh9t4hLApFrwb0ID8pUSOtt8VKmJ
| Content-Type: text/plain; charset=windows-1252
| Content-Language: en-US
| Content-Transfer-Encoding: quoted-printable
| 
| On 14.01.2016 21:41, Christos Zoulas wrote:
| > +The
| > +.Fa rm
| > +array must be at least 10 elements long, and should contain the result
| > +of the matches from a previous
| > +.Fn regexec
| > +call.
| 
| Could we have an argument to regasub(3)/regnsub(3) "size_t nmatch" like
| in regexec(3), instead of assuming >=3D 10 elements long?
| 
| It might not be too late to alter this function. There is only 1 user in
| GCC and no stable releases with this API.
| 
| My rationale is to sanitize these interfaces without caching the number
| of elements for a regexec(3) call in a sanitizer. Additionally we could
| have an internal sanity check to prevent out of bound operations on the
| "regmatch_t *" type.

Sure, fix it and pullup-8.

christos


Re: CVS commit: src/lib/libc/regex

2018-02-24 Thread Kamil Rytarowski
On 14.01.2016 21:41, Christos Zoulas wrote:
> +The
> +.Fa rm
> +array must be at least 10 elements long, and should contain the result
> +of the matches from a previous
> +.Fn regexec
> +call.

Could we have an argument to regasub(3)/regnsub(3) "size_t nmatch" like
in regexec(3), instead of assuming >= 10 elements long?

It might not be too late to alter this function. There is only 1 user in
GCC and no stable releases with this API.

My rationale is to sanitize these interfaces without caching the number
of elements for a regexec(3) call in a sanitizer. Additionally we could
have an internal sanity check to prevent out of bound operations on the
"regmatch_t *" type.



signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/sys/arch/amd64/amd64

2018-02-24 Thread Maxime Villard

Le 24/02/2018 à 17:30, Christos Zoulas a écrit :

In article <18bc2a5a-f82d-91ba-5e52-b262c907b...@m00nbsd.net>,
Maxime Villard   wrote:

Le 24/02/2018 à 11:54, Martin Husemann a écrit :

On Sat, Feb 24, 2018 at 11:37:11AM +0100, Maxime Villard wrote:

If the macro was defined as #if, you would need to do something like:

SYSCALL_ENTRY(syscall)
#define SYSCALL_ENTRY_SVS
SYSCALL_ENTRY(syscall_svs)
#undef SYSCALL_ENTRY_SVS

Where SYSCALL_ENTRY would contain another macro that depends on whether
SYSCALL_ENTRY_SVS is defined.


Not sure I follow here.

I would do something like:

SYSCALL_ENTRY_PLAIN(syscall)
SYSCALL_ENTRY_SVS(syscall_svs)

and have the SYSCALL_ENTRY_SVS be defined empty ifndef SVS.


But then you are duplicating the code that is shared between the two.


Yes, I can see why you prefer macros here, but you are also duplicating
the stack frame formation code just because in one branch you are using
r15 and in the other rax. Why not simplify it? or use a macro for it?


Actually I was unhappy about having two different branches too. But thinking
about this, now that we have a dynamic detection for SVS, we can use %rax in
both branches. I've fixed that in rev1.155, now there is no duplication.

Maxime


Re: CVS commit: src/sys/arch/amd64/amd64

2018-02-24 Thread Christos Zoulas
In article <18bc2a5a-f82d-91ba-5e52-b262c907b...@m00nbsd.net>,
Maxime Villard   wrote:
>Le 24/02/2018 à 11:54, Martin Husemann a écrit :
>> On Sat, Feb 24, 2018 at 11:37:11AM +0100, Maxime Villard wrote:
>>> If the macro was defined as #if, you would need to do something like:
>>>
>>> SYSCALL_ENTRY(syscall)
>>> #define SYSCALL_ENTRY_SVS
>>> SYSCALL_ENTRY(syscall_svs)
>>> #undef SYSCALL_ENTRY_SVS
>>>
>>> Where SYSCALL_ENTRY would contain another macro that depends on whether
>>> SYSCALL_ENTRY_SVS is defined.
>> 
>> Not sure I follow here.
>> 
>> I would do something like:
>> 
>> SYSCALL_ENTRY_PLAIN(syscall)
>> SYSCALL_ENTRY_SVS(syscall_svs)
>> 
>> and have the SYSCALL_ENTRY_SVS be defined empty ifndef SVS.
>
>But then you are duplicating the code that is shared between the two.

Yes, I can see why you prefer macros here, but you are also duplicating
the stack frame formation code just because in one branch you are using
r15 and in the other rax. Why not simplify it? or use a macro for it?

christos



Re: CVS commit: src/sys/arch/amd64/amd64

2018-02-24 Thread Maxime Villard

Le 24/02/2018 à 11:54, Martin Husemann a écrit :

On Sat, Feb 24, 2018 at 11:37:11AM +0100, Maxime Villard wrote:

If the macro was defined as #if, you would need to do something like:

SYSCALL_ENTRY(syscall)
#define SYSCALL_ENTRY_SVS
SYSCALL_ENTRY(syscall_svs)
#undef SYSCALL_ENTRY_SVS

Where SYSCALL_ENTRY would contain another macro that depends on whether
SYSCALL_ENTRY_SVS is defined.


Not sure I follow here.

I would do something like:

SYSCALL_ENTRY_PLAIN(syscall)
SYSCALL_ENTRY_SVS(syscall_svs)

and have the SYSCALL_ENTRY_SVS be defined empty ifndef SVS.


But then you are duplicating the code that is shared between the two.


Re: CVS commit: src/sys/arch/amd64/amd64

2018-02-24 Thread Martin Husemann
On Sat, Feb 24, 2018 at 11:37:11AM +0100, Maxime Villard wrote:
> If the macro was defined as #if, you would need to do something like:
> 
>   SYSCALL_ENTRY(syscall)
>   #define SYSCALL_ENTRY_SVS
>   SYSCALL_ENTRY(syscall_svs)
>   #undef SYSCALL_ENTRY_SVS
> 
> Where SYSCALL_ENTRY would contain another macro that depends on whether
> SYSCALL_ENTRY_SVS is defined.

Not sure I follow here.

I would do something like:

SYSCALL_ENTRY_PLAIN(syscall)
SYSCALL_ENTRY_SVS(syscall_svs)

and have the SYSCALL_ENTRY_SVS be defined empty ifndef SVS.

Martin


Re: CVS commit: src/sys/arch/amd64/amd64

2018-02-24 Thread Maxime Villard

Le 24/02/2018 à 11:14, Martin Husemann a écrit :

On Fri, Feb 23, 2018 at 08:09:09AM +0100, Maxime Villard wrote:

... And? There is only one place where we use .if instead of #if, because there
is a good reason for doing so.


Which reason is that?


Well, look at the code. We want to control what gets compiled in the macro
with an argument.

SYSCALL_ENTRY   syscall,is_svs=0
SYSCALL_ENTRY   syscall_svs,is_svs=1

If the macro was defined as #if, you would need to do something like:

SYSCALL_ENTRY(syscall)
#define SYSCALL_ENTRY_SVS
SYSCALL_ENTRY(syscall_svs)
#undef SYSCALL_ENTRY_SVS

Where SYSCALL_ENTRY would contain another macro that depends on whether
SYSCALL_ENTRY_SVS is defined.

The second approach is the one that complexifies the code.

Maxime


Re: CVS commit: src/sys/arch/amd64/amd64

2018-02-24 Thread Martin Husemann
On Fri, Feb 23, 2018 at 08:09:09AM +0100, Maxime Villard wrote:
> ... And? There is only one place where we use .if instead of #if, because 
> there
> is a good reason for doing so.

Which reason is that?

Martin


Re: CVS commit: src/sys/arch/amd64/amd64

2018-02-24 Thread Maxime Villard

Le 22/02/2018 à 17:31, Christos Zoulas a écrit :

In article <7f4de63c-e782-14e6-5554-9b9d23471...@m00nbsd.net>,
Maxime Villard   wrote:

Le 22/02/2018 à 15:54, Christos Zoulas a écrit :

In article <20180222140848.70e95f...@cvs.netbsd.org>,
Martin Husemann  wrote:

-=-=-=-=-=-

Module Name:src
Committed By:   martin
Date:   Thu Feb 22 14:08:48 UTC 2018

Modified Files:
src/sys/arch/amd64/amd64: locore.S

Log Message:
Protect the SVS part of SYSCALL_ENTRY by #ifdef SVS to make non-SVS
kernels compile again.


The combination of "#ifdef" and ".if" makes the code more horrific.
Can we use one and not the other? Preferrably "#ifdef" since we already
use it extensively?


In this case the ifdef just had to be put around the declaration.

You can't replace .if by #ifdef, there are two SYSCALL_ENTRY declarations,
and we give a different argument depending on whether we want the SVS code
to be in the macro or not.


The question is do we want to keep using both cpp and assembly macros.


Why wouldn't we? I don't see the problem.


The use of assembly macros is recent, the cpp one has always been there.
I.e. until recently we were not using .macro or .if, now we are.


... And? There is only one place where we use .if instead of #if, because there
is a good reason for doing so. It doesn't occur to me we need to replace all
the other #ifs by .ifs as a result.

Maxime