Re: [Tinycc-devel] fix: code in non-executable sections

[grischka]

On 01.08.2024 13:22, Jonathan M. Wilbur wrote:
> I am terribly sorry on both counts. I actually had no idea there even
> was a test suite. I thought the test folder was just an assortment of
> files to manually test against as needed.

If that were so then after 20 years there would be hundreds of files
like yours in "test", no?  Lots of files which nobody would know how
to run.

> I will fix this immediately. If it means anything, I built a few executables 
> already just fine with my changes: I think most people that have downloaded and 
> used TinyCC in the past few hours should not have had any problems, unless they 
> ran the tests.
>
> I actually already found out that the relocation was broken only because of my 
> change that made sections defined in assembly executable. (The change made in 
> gen_function seems to be fine.) As it turns out, it is because I unknowingly 
> also changed the behavior of the TOK_ASMDIR_pushsection case, and .pushsection 
> is used in the tests.
>
> To get to my point: should I fix the problem or fix the test?

Why don't you try to do something useful instead?  Such as parse section
flags,  say "w" and "x" at least?

> I did this in the first place because (I think) most use cases for
> hand-crafted assembly are for defining code, not data,

Keep in mind that changes to the public tinycc will affect everybody's
use cases, not just your own.

For example

   .section .data
hello_str:
   .string "hello"

> but it looks like tcctest.c uses .pushsection to define data-only sections. In 
> my opinion, I still think the behavior of my change is generally correct, but 
> it is not clear to me what the tests using pushsection are doing.
>
> While I await feedback, I will submit a change that fixes both issues.

Why not wait first and submit your final version then?

-- gr

> On Wed, Jul 31, 2024 at 06:52, Herman ten Brugge via Tinycc-devel 
> <tinycc-devel@nongnu.org> wrote:
> > On 7/30/24 16:46, Jonathan M. Wilbur wrote:
> > > Hello,
> > >
> > > I recently pushed a commit (e4d874d88a2ef874a9c2fd7c47d3d3a35ae986e2) that fixes code 
> > > being placed in ELF sections that do not have the executable bit set. This happens when a 
> > > function is annotated with __attribute__((section(".some_section"))), or when a 
> > > section is defined via inline assembly. In the latter case, it is assumed that the inline 
> > > assembly actually contains code, which will not always be the case, but should be the 
> > > case the vast majority of the time. When the instruction pointer points to code in a 
> > > non-executable section, a segmentation fault occurs.
> > >
> > > I have included two files that recreate both of these bugs in 
> > > tests/exec_section_in_c.c and tests/exec_section_in_asm.c. The patch is 
> > > included on this email.
> > >
> > > If anybody has any feedback, I would be curious to know if the blind addition 
> > > of the executable flag to sections defined in assembly will be a problem: 
> > > particularly if there is widespread usage of the writeable flag for these, 
> > > which might elicit segmentation faults from the combined use of the writeable 
> > > and executable flags for the same page.
> >
> > Same as previous mail:
> >
> > You probably did not run 'make test'.
> >
> > Tests should go into 'tests/tests2' directory.
> >
> > Some tests do not run.
> >
> > Looks like you have to revert the patch.
>
>
> _______________________________________________
> Tinycc-devel mailing list
> Tinycc-devel@nongnu.org
> https://lists.nongnu.org/mailman/listinfo/tinycc-devel