Hi!

As ugdbdos.zip\examples\gdbint\symify.pp is still broken and doesn't fully suite my needs, I'd like to rewrite some parts with focus on DOS, but I want to be sure, that this new version will be included with PP later. So do you have any advises for a fresh contributor?

Robert Riebisch

FPC includes the lineinfo unit to convert addresses into file and line information. The symify.pp is an old test example and deprecated.

Peter

On 3 Apr 08, at 20:39, Robert Riebisch wrote:

Hi Robert,

As ugdbdos.zip\examples\gdbint\symify.pp is still broken and doesn't fully suite my needs, I'd like to rewrite some parts with focus on DOS, but I want to be sure, that this new version will be included with PP later. So do you have any advises for a fresh contributor?

Considering that FPC is a multiplatform compiler, general advice is to think how to make the tool at least prepared for support of other platforms with similar needs. If a new tool is really needed (see response from Peter), it's probably useful to discuss the expectations and needs here to make sure the resulting tool is useable by other people having similar needs.

Tomas

Peter Vreman wrote

FPC includes the lineinfo unit to convert addresses into file and line information.

Thanks for the information. I didn't notice. I just build an application using "-gl" and I will get line number information on RTE, right?

The symify.pp is an old test example and deprecated.

So it will remain broken forever, because it's no longer needed?

Robert Riebisch

I think, symify.pp already is multi-platform and I didn't want to rewrite from scratch. ;-) But Peter seems to be right. Output from an application built with "-gl" should be enough for me. (Background: I'm trying to hunt a bug in FP's DOS IDE.)

Robert Riebisch

Oops, I obviously didn't read your e-mail properly. :-(

But Peter seems to be right. Output from an application built with "-gl" should be enough for me. (Background: I'm trying to hunt a bug in FP's DOS IDE.)

You're more than welcome. :-) What bug is it? You might try debugging with gdbpas too if just the line info is not sufficient.

Tomas

Tomas Hajny wrote

You're more than welcome. :-) What bug is it? You might try debugging with gdbpas too if just the line info is not sufficient.

I downloaded latest fpcbuild.zip and ran "make nogdb" in "ide" folder. This created a stripped fpc.exe. Where do I have to place "-gl"? In Makefile? In fpc.cfg?

Robert Riebisch

make "OPT=-gl" nogdb

fpc.cfg may or may not do depending on other parameters passed to make (like RELEASE=1 which results in -n option added, i.e. fpc.cfg is ignored completely then). BTW, with RELEASE=1 you'd need to extend the options to "OPT=-gl -Xs-", because -Xs is added for stripping otherwise.

Tomas

make OPR="-O- -gl" nogdb

is safer.

Jonas

Jonas Maebe wrote

> make "OPT=-gl" nogdb make OPR="-O- -gl" nogdb

`OPT=´, of course.

But it didn't help. Line info is simply not shown. :-(

So I built
<http://community.freepascal.org:10000/docs-html/user/userse57.html> for the go32v2 target. That's the result:

Runtime error 255 at $000020AC
$000020AC of ?·?
$000020B8 of ?·?
$000020C9 of ?·?
$00002098 of ?·?

Looks like the lineinfo unit is also broken.

Robert Riebisch

But it works for old FPC versions 2.0.4 (dos204.zip), 2.2.0 (my own build), or 2.2.1 (fpc-2.2.1.go32v2.zip).

Robert Riebisch

Parts of the lineinfo related code are rewritten in 2.3.1. There is no active development done on go32v2. So it can be that svn code is broken go32v2.

Peter

I see. Try to fix one bug, but find three others. ;-(

Robert Riebisch

I identified the problem now: "secofs" value returned by function "FindSectionCoff" in file "exeinfo.pp" is always 2,048 bytes too small. 2,048 (or $800) is the size of the go32 EXE stub.

I failed to find an elegant solution, that doesn't break PE32 or PE32PLUS. Maybe somebody else, who's more familiar with FPC's source code, could fix this?! Of course, I could assist as a tester.

Robert Riebisch

If nobody else can do it earlier, I'll try it, but probably not before end of this week.

Tomas

Thanks in advance! :-)

Robert Riebisch

I looked at it briefly, adding an offset of 2048 bytes is quite easy, one possible solution might be extending TExeFile record with yet another field (e.g. DebugOffset) and initialize this variable to 0 in all Open* functions except OpenGo32Coff, which should check for "go32stub" string at e_cparhdr * 16 (count of paragraphs in header * size of a paragraph = 10h) which is not done while reading but it supposedly should according to DJGPP sources for stubedit and set the value to 2048 in that case. BTW, this value of 2048 is already used there, it should be replaced with a constant if we go this direction.

However, I can't believe that information about this offset (i.e. size of the stub) isn't already available somewhere in the exe file itself (either in the general header or possibly in the COFF section headers), I just don't know where to look. :-( Knowing where it is might allow doing this in completely transparent manner - we could load information from the EXE file under all Coff platforms and already receive the right value for all of them. Obviously, one possible source of information might be DJGPP C sources for the gdb, but I'm not quite ready for this adventure... On the other hand, the previous version indeed simply added 2048 to all ofsets under GO32v2 (the code was completely GO32v2 specific then though), so actually the hack mentioned above at least wouldn't make it worse than it was in 2.2.x and before.

Tomas

Tomas Hajny wrote

However, I can't believe that information about this offset (i.e. size of the stub) isn't already available somewhere in the exe file itself (either in the general header or possibly in the COFF section headers), I just

The following simple snippet for computing "stubsize" is based on
function "create_go32_stub" in file "gnu\binutl-2.14\bfd\coff-stgo32.c"
from DJGPP's "bnu21?s.zip". Information is available here:
<http://www.delorie.com/djgpp/doc/exe/>. 512 bytes form a page (or
block) as per Microsoft's definition. "tdosheader" is from file
"exeinfo.pp".
***
type
  tdosheader = packed record
     e_magic : word;
     e_cblp : word;
     e_cp : word;
     e_crlc : word;
     e_cparhdr : word;
     e_minalloc : word;
     e_maxalloc : word;
     e_ss : word;
     e_sp : word;
     e_csum : word;
     e_ip : word;
     e_cs : word;
     e_lfarlc : word;
     e_ovno : word;
     e_res : array[0..3] of word;
     e_oemid : word;
     e_oeminfo : word;
     e_res2 : array[0..9] of word;
     e_lfanew : longint;
  end;
var
  f: file;
  dosheader: tdosheader;
  stubsize: cardinal; /* cardinal OK here? */
begin
  Assign(f, ParamStr(1));
  Reset(f, 1);
  BlockRead(f, dosheader, SizeOf(dosheader));
  if dosheader.e_magic = $5a4d then
  begin
    stubsize := dosheader.e_cp * 512;
    if dosheader.e_cblp > 0 then
      stubsize := stubsize + dosheader.e_cblp - 512;
    WriteLn('stub size: ', stubsize);
  end;
  Close(f);
end.
***

Robert Riebisch

.
.

Fix committed (and it seems to work according to first test - fp.exe started in directory without configuration file indeed provides nice backtrace on exit ;-) ).

Tomas

Tomas Hajny wrote

Fix committed (and it seems to work according to first test - fp.exe

You introduced "ParagraphSize", but subtract immediate value "512" in line 369. And thanks for giving credits. ;-)

started in directory without configuration file indeed provides nice backtrace on exit ;-) ).

I see, you have found "my" problem. :-)
Do you have any idea about that bug? I tracked the problem down to either procedure SetDefaultSwitches or writing OptimizingGoalSwitches to CfgFile, but have no clue.

Robert Riebisch

Ah, right, thanks for letting me know.

line 369. And thanks for giving credits. ;-)

Just giving credits where they're due. ;-)

I can't play with it now, but let me guess (all references below related to fpswitch.pas):

1) Previously, there've been two options for OptimizingGoalSwitches, but there's only one now (the other is not valid any longer) - line 1217.

2) SetDefaultSwitches calls OptimizingGoalSwitches^.SetCurrSel with index _1_ (note that index is 0-based), i.e. referring to _second_ value, in fact (there's no second value any longer, though - see above) - line 1438.

3) As opposed to SetBooleanItem, SetLongintItem or SetStringItem, SetCurrSel unfortunately doesn't check if the selected index isn't beyond the amount of items within the associated collection (using ItemCount further referring to Items^.Count).

4) When writing the contents down, IDE tries accessing a non-existing collection item. I really wonder how it comes that it doesn't bomb out on other targets, I believe that it certainly should...

If you track it down (either confirming my assumption above or finding some other reasons) and implement a good solution, a diff is the easiest way for me (or other core team members) to apply your fix. The problem probably exists for fixes branch too, so the fix should be eventually merged there as well.

Tomas

You did a good guess! :-)

some other reasons) and implement a good solution, a diff is the easiest

What is "a good solution"? :-)

I just changed line 1438 to "SetCurrSel(0)" and it worked for me. File "fp.cfg" was written properly and holds "-Os" as expected. But wait... How do we turn "-Os" off? It's a single radio button after "-OG" has been removed.

I prepared a patch (GNU diff) to remove "OptimizingGoalSwitches" completely and move "-Os" to "OptimizationSwitches": <http://www.bttr-software.de/tmp/fpide_os.zip> Please review it for mistakes, because I'm not very familiar with *Vision/OOP! :-)

Afterwards I realized "Level x optimizations" being check boxes. I think, that should be radio buttons instead. Furthermore "Position independent code" and "Create smartlinkable units" seem to be at the wrong place, because these are not "Run-time checks".

Robert Riebisch
P.S.: I signed up for bugs.freepascal.org several days ago, but never got an activation mail. :-(

Robert Riebisch wrote

Tomas Hajny wrote: > 1) Previously, there've been two options for OptimizingGoalSwitches, but

.
.

.
.

What is "a good solution"? :-)

Well, if you ask me, I'd say a good one should avoid similar problems in the future (i.e. fixing the SetCurrSel omission too as described above). ;-)

I just changed line 1438 to "SetCurrSel(0)" and it worked for me. File "fp.cfg" was written properly and holds "-Os" as expected. But wait... How do we turn "-Os" off? It's a single radio button after "-OG" has been removed.

I'd leave an answer for this to Jonas. The current situation doesn't make too much sense indeed.

I prepared a patch (GNU diff) to remove "OptimizingGoalSwitches" completely and move "-Os" to "OptimizationSwitches": <http://www.bttr-software.de/tmp/fpide_os.zip> Please review it for mistakes, because I'm not very familiar with *Vision/OOP! :-)

The patch looks OK (I haven't tested building, etc., but I assume you did). However, it would be probably useful to have some response from Jonas before applying it, maybe we don't need -Os at all now?

Afterwards I realized "Level x optimizations" being check boxes. I think, that should be radio buttons instead. Furthermore "Position independent code" and "Create smartlinkable units" seem to be at the wrong place, because these are not "Run-time checks".

Yes, this makes a lot of sense too.

P.S.: I signed up for bugs.freepascal.org several days ago, but never got an activation mail. :-(

Please, try it once more, just to make sure it wasn't some temporary one time issue. You should receive a response very quickly. If you don't, let us know, we'll forward this information to Marc so that he can have a look what's going on there (he is not subscribed to this list as far as I know).

Tomas

-Os and -OG cancel each other out. So the choice between them should either be a radio button, or a pop-up menu/selection box (or whatever the term is in *Vision). They are orthogonal to the other optimisation settings.

Jonas

Jonas Maebe wrote

-Os and -OG cancel each other out. So the choice between them should either be a radio button, or a pop-up menu/selection box (or whatever the term is in *Vision). They are orthogonal to the other optimisation settings.

Sure, but that's seems to be only true for FPC <= 2.2.x. FPC 2.3.1 doesn't list "-OG" anymore, but "-Os" only: "Optimize for size rather than speed".

So what to do? :-)

Robert Riebisch

Robert Riebisch wrote

Sure, but that's seems to be only true for FPC <= 2.2.x. FPC 2.3.1 doesn't list "-OG" anymore, but "-Os" only: "Optimize for size rather than speed".

Ah, I think Peter removed -OG. So now no -Os means "optimize for speed". There is no way anymore to turn off -Os after it's been turned on though, short of -O- (which completely resets all optimizer settings).

So turning -Os into a checkbox in the IDE should be ok (which you did in your patch as far as I understand).

Jonas

I see. :-)

Who removed "-OG" and why?

Yes, I did, of course. I'm happy, that I didn't forget all my Pascal knowledge, because I only looked through C code for the last two years.

did). However, it would be probably useful to have some response from Jonas before applying it, maybe we don't need -Os at all now?

Looks like he wants "-Os" AND "-OG" again. ;-) I welcome any decision as long as IDE looks consistently.

OK, but let's discuss first, before making another patch, which never will be applied. ;-)

> P.S.: I signed up for bugs.freepascal.org several days ago, but never > got an activation mail. :-( Please, try it once more, just to make sure it wasn't some temporary one

"That username is already being used." :-/

time issue. You should receive a response very quickly. If you don't, let

I also tried the "Lost password?" button, which tells me "Password Message Sent", but I don't get an e-mail.

us know, we'll forward this information to Marc so that he can have a look what's going on there (he is not subscribed to this list as far as I know).

Please do so. :-)

Robert Riebisch

I see.

So turning -Os into a checkbox in the IDE should be ok (which you did in your patch as far as I understand).

Yes, that's what the patch is for. :-)

Robert Riebisch

Robert Riebisch wrote

Tomas Hajny wrote:

.
.

.
.

Well, having radio buttons or checkboxes doesn't matter in fact in this particular case (any combination of -Ox will processed by the compiler equally as if just passing the highest used level regardless of the order). ;-) Admittedly, it's still confusing, because it looks like level 1 may not be included in level 2 (whereas it is), so changing to radio buttons would make sense, IMHO.

I guess this "wrong placement" can be fixed either by changing the title to something like "Code generation" or "Generated code features" (to avoid repeating the title used for that options "page"), or by creating another set of checkboxes (and thus another TSwitches object). I personally have no preference, although keeping aligned to grouping used in FPC itself (and thus rather renaming than creating another group) may make sense.

BTW, shortcut keys should be probably rearranged to avoid having 'm' assigned to two different options on that options page.

> > P.S.: I signed up for bugs.freepascal.org several days ago, but never > > got an activation mail. :-(

.
.

I made a small test and I know the reason now. However, the problem cannot be fixed by Marc or anybody here. The following happens to all e-mails sent from the server where the bug tracker runs (note that this is a different server from that one running the mailing lists, that's why you e-mails sent via mailing list are delivered OK):

: host mailin.dcpserver.de[80.252.97.204] said:
550-AF-Rejected: Sending from dialup IPs forbidden [ 550-http://postmaster.artfiles.de/BL-DS?id=0418-IYHyjmBia ] 550-************************************************************** 550-* see http://postmaster.artfiles.de/BL-DS?id=0418-IYHyjmBia * 550 ************************************************************** (in reply to RCPT TO command)

I'm afraid that you need to discuss this with administrator of your e-mail server, I certainly don't think that our server is connected via dialup...

Tomas

It's because the idefix' canonical hostname (the one you get when performing a reverse dns lookup on its ip address) contains "adsl", which is automatically interpreted as "dial-up/dynamic ip address" by many servers.

There are more providers blocking mails from idefix for probably that reason, such as pochta.ru (which also handles email for several other .ru domains including rbcmail.ru, hotbox.ru and front.ru), web.de and ukr.net.

Jonas

Robert Riebisch wrote

Jonas Maebe wrote:

.
.

> So turning -Os into a checkbox in the IDE should be ok (which you did > in your patch as far as I understand). Yes, that's what the patch is for. :-)

The patch is applied now. I extended it with fix for error checking in SetCurrSel, fix of conflicting shortcut keys in the compiler dialogue and changes related to the misleading label "run-time checks". The only thing I didn't touch are the optimization levels as check boxes rather than radio buttons. r10708 should be merged to fixes_2_2 in few days if everything works OK.

Tomas