Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

access() fix #86

Merged
merged 4 commits into from
Jun 23, 2020
Merged

access() fix #86

merged 4 commits into from
Jun 23, 2020

Conversation

bucanero
Copy link

thanks to @crystalct 's feedback, we detected that sysLv2FsAccess() crashes and halts the PS3 system

Based on code he shared, I'm proposing this access() workaround that doesn't use the lv2 syscall, while keeping the expected behavior.

bucanero added 2 commits June 22, 2020 11:23
sysLv2FsAccess() crashes and halts the system :-(
this is a workaround that doesn't use the lv2 syscall
by definition, the runtime library shouldn't reset errno to 0.
@zeldin
Copy link
Member

zeldin commented Jun 22, 2020

I don't particularly like this workaround. If sysLv2FsAccess() crashes shouldn't it be fixed instead? Maybe the prototype is just wrong or something? Where did the information about lv2 syscall 816 come from?

@crystalct
Copy link

https://www.psdevwiki.com/ps3/LV2_Functions_and_Syscalls
But, if you look inside Sony sdk libfs reference, there is no cellFSAccess function. Maybe it has never been implemented.

@zeldin
Copy link
Member

zeldin commented Jun 22, 2020

Maybe of interest is #63 from 6 years ago, which also says access didn't work out...

Anyway, if sysLv2FsAccess() doesn't work we should just remove it.

As for access() this is a bit confusing to me. As far as I can see, newlib does not provide access() at all, so there is no access_r field in struct __syscalls_t. Does the code added by #74 even compile?

@zeldin
Copy link
Member

zeldin commented Jun 22, 2020

Ah, there is a change to patches/newlib-1.20.0-PS3.patch in ps3toolchain which has no corresponding change in newlib-1.20.0-PS3...
My first suggestion would be to back this out again. access() is not a standard C function, so unless there is some system functionality backing it, there doesn't seem to be any need to have it.
If there is a compelling reason to have an emulated access() anyway, I'd suggest to put the emulation directly in newlib, not in PSL1GHT.

@zeldin
Copy link
Member

zeldin commented Jun 22, 2020

For reference, there is an earlier case of emulating access() using stat() inside newlib already:
newlib/libc/sys/sysmec/access.c

@bucanero
Copy link
Author

hello @zeldin , thanks for the feedback

Maybe the prototype is just wrong or something? Where did the information about lv2 syscall 816 come from?

As mentioned by @crystalct , the info was taken from the psdevwiki syscall page

I also checked the RPCS3 emu source, and they have a similar definition for sys_fs_access (syscall 816, using 2 parameters path and mode):
https://github.com/RPCS3/rpcs3/blob/master/rpcs3/Emu/Cell/lv2/lv2.cpp
https://github.com/RPCS3/rpcs3/blob/master/rpcs3/Emu/Cell/lv2/sys_fs.h
https://github.com/RPCS3/rpcs3/blob/master/rpcs3/Emu/Cell/lv2/sys_fs.cpp

but again, it's not working in real hardware. (perhaps it was removed at some point)

Maybe of interest is #63 from 6 years ago, which also says access didn't work out...

Anyway, if sysLv2FsAccess() doesn't work we should just remove it.

alright, so sysLv2FsAccess() should be removed from sys/file.h , right?

As for access() this is a bit confusing to me. As far as I can see, newlib does not provide access() at all, so there is no access_r field in struct __syscalls_t. Does the code added by #74 even compile?

it compiles, but requires the updated newlib patch that adds access_r to the struct:
ps3dev/ps3toolchain@c01ba0c

@bucanero
Copy link
Author

Ah, there is a change to patches/newlib-1.20.0-PS3.patch in ps3toolchain which has no corresponding change in newlib-1.20.0-PS3...
My first suggestion would be to back this out again. access() is not a standard C function, so unless there is some system functionality backing it, there doesn't seem to be any need to have it.
If there is a compelling reason to have an emulated access() anyway, I'd suggest to put the emulation directly in newlib, not in PSL1GHT.

Alright, if understand correctly, the rationale is to keep code in PSL1GHT if it's backed by some PS3 system functionality. If not, it should go to newlib.

I think that having the access() function, even if it's an emulated version, is still nice to have. (e.g., sometimes you're trying to port a library or piece of code that uses it, and even if it can be manually patched, it can introduce errors)

Back on this topic, which changes should I focus on?

  • sysLv2FsAccess() should be removed from sys/file.h
  • remove librt/access.c from PSL1GHT
  • newlib patch modification, to implement the emulated access() in access.c

did I miss something, or got something wrong?

thanks

@miigotu miigotu merged commit cab525d into ps3dev:master Jun 23, 2020
@bucanero
Copy link
Author

bucanero commented Jun 23, 2020

I'll try to propose a newlib patch with the emulated access() for review (on the ps3toolchain repo)

edit: newlib emulated access(), ps3dev/ps3toolchain#91

@bucanero bucanero deleted the access-fix branch June 23, 2020 15:26
@zeldin
Copy link
Member

zeldin commented Jun 24, 2020

Alright, if understand correctly, the rationale is to keep code in PSL1GHT if it's backed by some PS3 system functionality. If not, it should go to newlib.

Yes. __syscalls exists in order for newlib to be able to call lv2 without having to include all the magic sauce in the newlib proper. Things which are implemented using other newlib functions or just stubbed can go directly into newlib to avoid the roundtrip. See for example libgloss/libsysbase/abort.c, liibgloss/libsysbase/environ.c and libgloss/libsysbase/glob.c.

  • sysLv2FsAccess() should be removed from sys/file.h
  • remove librt/access.c from PSL1GHT
  • newlib patch modification, to implement the emulated access() in access.c

Yes, and access_r removed from __syscalls_t as part of your last point. Needs to happen after access.c is removed from PSL1GHT to avoid breaking the build.

Please make sure that the newlib-1.20.0-PS3 repo and the patch in ps3toolchain are kept in sync.

Thanks

@zeldin
Copy link
Member

zeldin commented Jun 24, 2020

As mentioned by @crystalct , the info was taken from the psdevwiki syscall page

I also checked the RPCS3 emu source, and they have a similar definition for sys_fs_access (syscall 816, using 2 parameters path and mode):
https://github.com/RPCS3/rpcs3/blob/master/rpcs3/Emu/Cell/lv2/lv2.cpp
https://github.com/RPCS3/rpcs3/blob/master/rpcs3/Emu/Cell/lv2/sys_fs.h
https://github.com/RPCS3/rpcs3/blob/master/rpcs3/Emu/Cell/lv2/sys_fs.cpp

but again, it's not working in real hardware. (perhaps it was removed at some point)

Well, that is not an official Sony document. Presumably the information comes either from a leak or from an RE effort, but either way we don't have direct access to the source so we can't check if a mistake was made on the wiki. Presumably the RPCS3 authors also blindly believed the wiki page. 😄

@bucanero
Copy link
Author

hello @zeldin , thanks for the explanation about newlib __syscalls and the logic behind what-goes-where. 😄

Indeed, as you said, RPCS3 is not official and they might have also based their work on the wiki docs or some dump/leak/RE. Since @crystalct mentioned that there's no reference about it in the official SDK docs, I guess it's safe to assume that the function is not available.
(Still funny to see how an error in a wiki document can propagate across many different projects)

Newlib PRs:
ps3dev/ps3toolchain#91
ps3dev/newlib-1.20.0-PS3#1

@crystalct
Copy link

About cellFSAccess and RPCS3:
RPCS3/rpcs3#8492 (comment)

@crystalct
Copy link

@zeldin @bucanero @wargio @miigotu
I have installed all from scratch. Tested wargio/ps3soundlib and wargio/tiny3d ... almost perfect.
Raw2h is missing and it is necessary to build ps3soundlib. Installing it before ps3soundlib it was perfect.

@crystalct
Copy link

Also unzip and rhash are necessary (cgcomp).

@wargio
Copy link

wargio commented Jun 25, 2020

@zeldin @bucanero @wargio @miigotu
I have installed all from scratch. Tested wargio/ps3soundlib and wargio/tiny3d ... almost perfect.
Raw2h is missing and it is necessary to build ps3soundlib. Installing it before ps3soundlib it was perfect.

raw2h is now raw2s

@wargio
Copy link

wargio commented Jun 25, 2020

hello @zeldin , thanks for the feedback

Maybe the prototype is just wrong or something? Where did the information about lv2 syscall 816 come from?

As mentioned by @crystalct , the info was taken from the psdevwiki syscall page

I also checked the RPCS3 emu source, and they have a similar definition for sys_fs_access (syscall 816, using 2 parameters path and mode):
https://github.com/RPCS3/rpcs3/blob/master/rpcs3/Emu/Cell/lv2/lv2.cpp
https://github.com/RPCS3/rpcs3/blob/master/rpcs3/Emu/Cell/lv2/sys_fs.h
https://github.com/RPCS3/rpcs3/blob/master/rpcs3/Emu/Cell/lv2/sys_fs.cpp

but again, it's not working in real hardware. (perhaps it was removed at some point)

Maybe of interest is #63 from 6 years ago, which also says access didn't work out...
Anyway, if sysLv2FsAccess() doesn't work we should just remove it.

alright, so sysLv2FsAccess() should be removed from sys/file.h , right?

As for access() this is a bit confusing to me. As far as I can see, newlib does not provide access() at all, so there is no access_r field in struct __syscalls_t. Does the code added by #74 even compile?

it compiles, but requires the updated newlib patch that adds access_r to the struct:
ps3dev/ps3toolchain@c01ba0c

which os version are you using?

@bucanero
Copy link
Author

which os version are you using?

I'm using macOS (10.15.5)

@wargio
Copy link

wargio commented Jun 25, 2020

which os version are you using?

I'm using macOS (10.15.5)

no i mean on the ps3.

@bucanero
Copy link
Author

I'm on 4.84 firmware.

@wargio
Copy link

wargio commented Jun 25, 2020

I'm on 4.84 firmware.

hmm i should try this on my 3.55 ps3.

@crystalct
Copy link

@zeldin @bucanero @wargio @miigotu
I have installed all from scratch. Tested wargio/ps3soundlib and wargio/tiny3d ... almost perfect.
Raw2h is missing and it is necessary to build ps3soundlib. Installing it before ps3soundlib it was perfect.

raw2h is now raw2s

On windows raw2h must be compiled and installed. raw2s doesn't exist... i suppose they are the same.....

@crystalct
Copy link

hello @zeldin , thanks for the feedback

Maybe the prototype is just wrong or something? Where did the information about lv2 syscall 816 come from?

As mentioned by @crystalct , the info was taken from the psdevwiki syscall page
I also checked the RPCS3 emu source, and they have a similar definition for sys_fs_access (syscall 816, using 2 parameters path and mode):
https://github.com/RPCS3/rpcs3/blob/master/rpcs3/Emu/Cell/lv2/lv2.cpp
https://github.com/RPCS3/rpcs3/blob/master/rpcs3/Emu/Cell/lv2/sys_fs.h
https://github.com/RPCS3/rpcs3/blob/master/rpcs3/Emu/Cell/lv2/sys_fs.cpp
but again, it's not working in real hardware. (perhaps it was removed at some point)

Maybe of interest is #63 from 6 years ago, which also says access didn't work out...
Anyway, if sysLv2FsAccess() doesn't work we should just remove it.

alright, so sysLv2FsAccess() should be removed from sys/file.h , right?

As for access() this is a bit confusing to me. As far as I can see, newlib does not provide access() at all, so there is no access_r field in struct __syscalls_t. Does the code added by #74 even compile?

it compiles, but requires the updated newlib patch that adds access_r to the struct:
ps3dev/ps3toolchain@c01ba0c

which os version are you using?

RPCS3 guys claim that cellFsAccess (its syscal 816) exist. They found it using reverse engineering ....
RPCS3/rpcs3#8492

@bucanero
Copy link
Author

bucanero commented Jun 25, 2020

I'm on 4.84 firmware.

hmm i should try this on my 3.55 ps3.

Yes, at least for information purposes, It would be nice to know if the lv2 access() was available on 3.55 😄

@bucanero
Copy link
Author

raw2h is now raw2s

On windows raw2h must be compiled and installed. raw2s doesn't exist... i suppose they are the same.....

it seems that raw2h was originally available as part of the PS1LIGHT tools: https://github.com/HACKERCHANNEL/PSL1GHT/blob/master/tools/raw2h/raw2h.c

the only code I could find about raw2s is here:
https://github.com/mikeryan/n64dev/blob/master/util/n64tools/raw2s.c

I guess we could just re-add the original raw2h.c to the tools folder.

@wargio
Copy link

wargio commented Jun 25, 2020

I don't see the need. raw2s is the new raw2h. Essentially it generates a header and a assembly file.

@bucanero
Copy link
Author

I don't see the need. raw2s is the new raw2h. Essentially it generates a header and a assembly file.

you are referring as if raw2s was available by default, but I don't see raw2s in PSL1GHT nor in the ps3toolchain.
Can you point where is it available?

@miigotu
Copy link
Member

miigotu commented Jun 26, 2020

raw2h was probably broken somehow, but it should be available for legacy projects that still call it.

@crystalct
Copy link

Is raw2s a standard linux command?

@crystalct
Copy link

@wargio you added check on VPCOMP and RAW2H variables to compile Tini3d and both variable doesn't exist inside psl1ght ppu_rules. CGCOMP exist, probably they are the same util. In the installation process they must be included and to check/install raw2h/s utility and an eventual vpcom utility.

@crystalct
Copy link

Only one raw2s that i found with google is inside n64tools-1-1 (nintendo 64 tools).

@miigotu
Copy link
Member

miigotu commented Jun 26, 2020

image

@crystalct
Copy link

I propose raw2h present in the old psl1ght as "offical" (when and where it is needed)

@miigotu
Copy link
Member

miigotu commented Jun 26, 2020

e573ba2

@wargio
Copy link

wargio commented Jun 26, 2020

e573ba2

Again, why you adding it back? https://github.com/ps3dev/PSL1GHT/blob/master/base_rules#L89

@crystalct
Copy link

@wargio do you meen raw2h is bin2s?

@wargio
Copy link

wargio commented Jun 26, 2020

you can just check how bin2s works: https://github.com/wargio/NoRSX/blob/master/NoRSX_Example/Makefile#L147

@wargio
Copy link

wargio commented Jun 26, 2020

@wargio do you meen raw2h is bin2s?

yes it is.

@crystalct
Copy link

ohhhh good ^_^
About VPCOMP & CGCOMP?

@crystalct
Copy link

@wargio @miigotu
I compiled Tiny3D and samples with:
export RAW2H=/usr/local/ps3dev/bin/bin2s.exe
export VPCOMP=/usr/local/ps3dev/bin/cgcomp.exe
So, at this point, i suggest to add RAW2H (@miigotu ) inside ppu_rules and change VPCOMP to CGCOMP (@wargio)

@wargio
Copy link

wargio commented Jun 26, 2020

@crystalct i think we just need to define these targets in the makefile https://github.com/ps3dev/PSL1GHT/blob/master/ppu_rules#L72

@crystalct
Copy link

@wargio , i mean that.... just about VPCOMP & CGCOMP.... do we need both?

@wargio
Copy link

wargio commented Jun 26, 2020

@wargio , i mean that.... just about VPCOMP & CGCOMP.... do we need both?

they are synonym. so a PR is enough

@crystalct
Copy link

Now VPCOMP is CGCOMP
PR done for RAW2H inside ppu_rules

@miigotu
Copy link
Member

miigotu commented Jun 26, 2020

e573ba2

Again, why you adding it back? https://github.com/ps3dev/PSL1GHT/blob/master/base_rules#L89

It isn't hurting anything to be there in the tools folder. That way if any legacy software is calling for it you just compile it and be on your way.

@wargio
Copy link

wargio commented Jun 26, 2020

can i suggest to have a CI?

@bucanero
Copy link
Author

@miigotu has been working on a workflow to build a Docker image for ps3toolchain/psl1ght using submodules

on my fork I've made a simple workflow to build binaries for Ubuntu and macOS:
https://github.com/bucanero/ps3toolchain/blob/master/.github/workflows/build.yml

@miigotu
Copy link
Member

miigotu commented Jun 27, 2020

Toolchain builds fine right now. Having some issue with my workflow right now, but there will be multi-arch docker builds and releases for win/Mac/Linux autocreated soon. If it builds it passes.

@crystalct
Copy link

@miigotu
It's seems rsxtest sample and all tiny3d samples hangs on real PS3. Black screen....
I will investigate better...

@miigotu
Copy link
Member

miigotu commented Jun 27, 2020

I'm thinking I pushed that vectormath commit that didn't belong. Can't remember if I put it up on master before lol. I'll revert that commit later, but you could try that @crystalct

@zeldin
Copy link
Member

zeldin commented Jun 27, 2020

Why has the comment section of this PR turned into an IRC channel? 😕

@miigotu
Copy link
Member

miigotu commented Jun 27, 2020

Ha, I'm on both freenode and efnet same name if anyone needs to chat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants